* [PATCH v3 01/10] drm/fb-helper: Cleanup checkpatch warnings
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 9:36 ` Daniel Vetter
2017-03-21 8:13 ` [PATCH v3 02/10] drm/fb-helper: Reshuffle code for subsequent patches Thierry Reding
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
Fix up a couple of checkpatch warnings, such as whitespace or coding
style issues.
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_fb_helper.c | 54 ++++++++++++++++++++++++-----------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 45fde2ffef2a..2be1cd949cec 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -213,9 +213,9 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
fb_helper_connector = fb_helper->connector_info[i];
drm_connector_put(fb_helper_connector->connector);
- for (j = i + 1; j < fb_helper->connector_count; j++) {
+ for (j = i + 1; j < fb_helper->connector_count; j++)
fb_helper->connector_info[j - 1] = fb_helper->connector_info[j];
- }
+
fb_helper->connector_count--;
kfree(fb_helper_connector);
@@ -316,6 +316,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
for (i = 0; i < helper->crtc_count; i++) {
struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set;
+
crtc = mode_set->crtc;
funcs = crtc->helper_private;
fb = drm_mode_config_fb(crtc);
@@ -346,7 +347,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
struct drm_plane *plane;
struct drm_atomic_state *state;
int i, ret;
- unsigned plane_mask;
+ unsigned int plane_mask;
state = drm_atomic_state_alloc(dev);
if (!state)
@@ -378,7 +379,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
goto fail;
}
- for(i = 0; i < fb_helper->crtc_count; i++) {
+ for (i = 0; i < fb_helper->crtc_count; i++) {
struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
ret = __drm_atomic_helper_set_config(mode_set, state);
@@ -488,8 +489,10 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
struct drm_crtc *crtc;
int bound = 0, crtcs_bound = 0;
- /* Sometimes user space wants everything disabled, so don't steal the
- * display if there's a master. */
+ /*
+ * Sometimes user space wants everything disabled, so don't steal the
+ * display if there's a master.
+ */
if (READ_ONCE(dev->master))
return false;
@@ -537,6 +540,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
static void drm_fb_helper_restore_work_fn(struct work_struct *ignored)
{
bool ret;
+
ret = drm_fb_helper_force_kernel_mode();
if (ret == true)
DRM_ERROR("Failed to restore crtc configuration\n");
@@ -870,9 +874,8 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
mutex_lock(&kernel_fb_helper_lock);
if (!list_empty(&fb_helper->kernel_fb_list)) {
list_del(&fb_helper->kernel_fb_list);
- if (list_empty(&kernel_fb_helper_list)) {
+ if (list_empty(&kernel_fb_helper_list))
unregister_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
- }
}
mutex_unlock(&kernel_fb_helper_lock);
@@ -1165,6 +1168,7 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
(blue << info->var.blue.offset);
if (info->var.transp.length > 0) {
u32 mask = (1 << info->var.transp.length) - 1;
+
mask <<= info->var.transp.offset;
value |= mask;
}
@@ -1447,7 +1451,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
struct drm_atomic_state *state;
struct drm_plane *plane;
int i, ret;
- unsigned plane_mask;
+ unsigned int plane_mask;
state = drm_atomic_state_alloc(dev);
if (!state)
@@ -1456,7 +1460,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
state->acquire_ctx = dev->mode_config.acquire_ctx;
retry:
plane_mask = 0;
- for(i = 0; i < fb_helper->crtc_count; i++) {
+ for (i = 0; i < fb_helper->crtc_count; i++) {
struct drm_mode_set *mode_set;
mode_set = &fb_helper->crtc_info[i].mode_set;
@@ -1561,11 +1565,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
sizes.surface_depth = 24;
sizes.surface_bpp = 32;
- sizes.fb_width = (unsigned)-1;
- sizes.fb_height = (unsigned)-1;
+ sizes.fb_width = (u32)-1;
+ sizes.fb_height = (u32)-1;
- /* if driver picks 8 or 16 by default use that
- for both depth/bpp */
+ /* if driver picks 8 or 16 by default use that for both depth/bpp */
if (preferred_bpp != sizes.surface_bpp)
sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
@@ -1630,6 +1633,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
for (j = 0; j < mode_set->num_connectors; j++) {
struct drm_connector *connector = mode_set->connectors[j];
+
if (connector->has_tile) {
lasth = (connector->tile_h_loc == (connector->num_h_tile - 1));
lastv = (connector->tile_v_loc == (connector->num_v_tile - 1));
@@ -1645,8 +1649,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
}
if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
- /* hmm everyone went away - assume VGA cable just fell out
- and will come back later. */
+ /*
+ * hmm everyone went away - assume VGA cable just fell out
+ * and will come back later.
+ */
DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
sizes.fb_width = sizes.surface_width = 1024;
sizes.fb_height = sizes.surface_height = 768;
@@ -1703,7 +1709,6 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
info->fix.accel = FB_ACCEL_NONE;
info->fix.line_length = pitch;
- return;
}
EXPORT_SYMBOL(drm_fb_helper_fill_fix);
@@ -1725,6 +1730,7 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
uint32_t fb_width, uint32_t fb_height)
{
struct drm_framebuffer *fb = fb_helper->fb;
+
info->pseudo_palette = fb_helper->pseudo_palette;
info->var.xres_virtual = fb->width;
info->var.yres_virtual = fb->height;
@@ -2057,13 +2063,15 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
continue;
} else {
- if (fb_helper_conn->connector->tile_h_loc != tile_pass -1 &&
+ if (fb_helper_conn->connector->tile_h_loc != tile_pass - 1 &&
fb_helper_conn->connector->tile_v_loc != tile_pass - 1)
/* if this tile_pass doesn't cover any of the tiles - keep going */
continue;
- /* find the tile offsets for this pass - need
- to find all tiles left and above */
+ /*
+ * find the tile offsets for this pass - need to find
+ * all tiles left and above
+ */
drm_get_tile_offsets(fb_helper, modes, offsets,
i, fb_helper_conn->connector->tile_h_loc, fb_helper_conn->connector->tile_v_loc);
}
@@ -2147,8 +2155,10 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
if (!encoder)
goto out;
- /* select a crtc for this connector and then attempt to configure
- remaining connectors */
+ /*
+ * select a crtc for this connector and then attempt to configure
+ * remaining connectors
+ */
for (c = 0; c < fb_helper->crtc_count; c++) {
crtc = &fb_helper->crtc_info[c];
--
2.12.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 01/10] drm/fb-helper: Cleanup checkpatch warnings
2017-03-21 8:13 ` [PATCH v3 01/10] drm/fb-helper: Cleanup checkpatch warnings Thierry Reding
@ 2017-03-21 9:36 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 9:36 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, John Stultz, dri-devel
On Tue, Mar 21, 2017 at 09:13:49AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Fix up a couple of checkpatch warnings, such as whitespace or coding
> style issues.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 54 ++++++++++++++++++++++++-----------------
> 1 file changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 45fde2ffef2a..2be1cd949cec 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -213,9 +213,9 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> fb_helper_connector = fb_helper->connector_info[i];
> drm_connector_put(fb_helper_connector->connector);
>
> - for (j = i + 1; j < fb_helper->connector_count; j++) {
> + for (j = i + 1; j < fb_helper->connector_count; j++)
> fb_helper->connector_info[j - 1] = fb_helper->connector_info[j];
> - }
> +
> fb_helper->connector_count--;
> kfree(fb_helper_connector);
>
> @@ -316,6 +316,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
>
> for (i = 0; i < helper->crtc_count; i++) {
> struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set;
> +
> crtc = mode_set->crtc;
> funcs = crtc->helper_private;
> fb = drm_mode_config_fb(crtc);
> @@ -346,7 +347,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
> struct drm_plane *plane;
> struct drm_atomic_state *state;
> int i, ret;
> - unsigned plane_mask;
> + unsigned int plane_mask;
>
> state = drm_atomic_state_alloc(dev);
> if (!state)
> @@ -378,7 +379,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
> goto fail;
> }
>
> - for(i = 0; i < fb_helper->crtc_count; i++) {
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
>
> ret = __drm_atomic_helper_set_config(mode_set, state);
> @@ -488,8 +489,10 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> struct drm_crtc *crtc;
> int bound = 0, crtcs_bound = 0;
>
> - /* Sometimes user space wants everything disabled, so don't steal the
> - * display if there's a master. */
> + /*
> + * Sometimes user space wants everything disabled, so don't steal the
> + * display if there's a master.
> + */
> if (READ_ONCE(dev->master))
> return false;
>
> @@ -537,6 +540,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
> static void drm_fb_helper_restore_work_fn(struct work_struct *ignored)
> {
> bool ret;
> +
> ret = drm_fb_helper_force_kernel_mode();
> if (ret == true)
> DRM_ERROR("Failed to restore crtc configuration\n");
> @@ -870,9 +874,8 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
> mutex_lock(&kernel_fb_helper_lock);
> if (!list_empty(&fb_helper->kernel_fb_list)) {
> list_del(&fb_helper->kernel_fb_list);
> - if (list_empty(&kernel_fb_helper_list)) {
> + if (list_empty(&kernel_fb_helper_list))
> unregister_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> - }
> }
> mutex_unlock(&kernel_fb_helper_lock);
>
> @@ -1165,6 +1168,7 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> (blue << info->var.blue.offset);
> if (info->var.transp.length > 0) {
> u32 mask = (1 << info->var.transp.length) - 1;
> +
> mask <<= info->var.transp.offset;
> value |= mask;
> }
> @@ -1447,7 +1451,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
> struct drm_atomic_state *state;
> struct drm_plane *plane;
> int i, ret;
> - unsigned plane_mask;
> + unsigned int plane_mask;
>
> state = drm_atomic_state_alloc(dev);
> if (!state)
> @@ -1456,7 +1460,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
> state->acquire_ctx = dev->mode_config.acquire_ctx;
> retry:
> plane_mask = 0;
> - for(i = 0; i < fb_helper->crtc_count; i++) {
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> struct drm_mode_set *mode_set;
>
> mode_set = &fb_helper->crtc_info[i].mode_set;
> @@ -1561,11 +1565,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
> sizes.surface_depth = 24;
> sizes.surface_bpp = 32;
> - sizes.fb_width = (unsigned)-1;
> - sizes.fb_height = (unsigned)-1;
> + sizes.fb_width = (u32)-1;
> + sizes.fb_height = (u32)-1;
>
> - /* if driver picks 8 or 16 by default use that
> - for both depth/bpp */
> + /* if driver picks 8 or 16 by default use that for both depth/bpp */
> if (preferred_bpp != sizes.surface_bpp)
> sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
>
> @@ -1630,6 +1633,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>
> for (j = 0; j < mode_set->num_connectors; j++) {
> struct drm_connector *connector = mode_set->connectors[j];
> +
> if (connector->has_tile) {
> lasth = (connector->tile_h_loc == (connector->num_h_tile - 1));
> lastv = (connector->tile_v_loc == (connector->num_v_tile - 1));
> @@ -1645,8 +1649,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> }
>
> if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
> - /* hmm everyone went away - assume VGA cable just fell out
> - and will come back later. */
> + /*
> + * hmm everyone went away - assume VGA cable just fell out
> + * and will come back later.
> + */
> DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
> sizes.fb_width = sizes.surface_width = 1024;
> sizes.fb_height = sizes.surface_height = 768;
> @@ -1703,7 +1709,6 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> info->fix.accel = FB_ACCEL_NONE;
>
> info->fix.line_length = pitch;
> - return;
> }
> EXPORT_SYMBOL(drm_fb_helper_fill_fix);
>
> @@ -1725,6 +1730,7 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
> uint32_t fb_width, uint32_t fb_height)
> {
> struct drm_framebuffer *fb = fb_helper->fb;
> +
> info->pseudo_palette = fb_helper->pseudo_palette;
> info->var.xres_virtual = fb->width;
> info->var.yres_virtual = fb->height;
> @@ -2057,13 +2063,15 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> continue;
>
> } else {
> - if (fb_helper_conn->connector->tile_h_loc != tile_pass -1 &&
> + if (fb_helper_conn->connector->tile_h_loc != tile_pass - 1 &&
> fb_helper_conn->connector->tile_v_loc != tile_pass - 1)
> /* if this tile_pass doesn't cover any of the tiles - keep going */
> continue;
>
> - /* find the tile offsets for this pass - need
> - to find all tiles left and above */
> + /*
> + * find the tile offsets for this pass - need to find
> + * all tiles left and above
> + */
> drm_get_tile_offsets(fb_helper, modes, offsets,
> i, fb_helper_conn->connector->tile_h_loc, fb_helper_conn->connector->tile_v_loc);
> }
> @@ -2147,8 +2155,10 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> if (!encoder)
> goto out;
>
> - /* select a crtc for this connector and then attempt to configure
> - remaining connectors */
> + /*
> + * select a crtc for this connector and then attempt to configure
> + * remaining connectors
> + */
> for (c = 0; c < fb_helper->crtc_count; c++) {
> crtc = &fb_helper->crtc_info[c];
>
> --
> 2.12.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 02/10] drm/fb-helper: Reshuffle code for subsequent patches
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
2017-03-21 8:13 ` [PATCH v3 01/10] drm/fb-helper: Cleanup checkpatch warnings Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 9:36 ` Daniel Vetter
2017-03-21 8:13 ` [PATCH v3 03/10] drm/fb-helper: Improve code readability Thierry Reding
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
An unlocked version of the drm_fb_helper_add_one_connector() function
will be added in a subsequent patch. Reshuffle the code separately to
make the diff more readable later on.
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_fb_helper.c | 59 ++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 2be1cd949cec..875a49048222 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -109,6 +109,35 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
i__ = 0; i__ < (fbh)->connector_count; i__++)
+int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector)
+{
+ struct drm_fb_helper_connector **temp;
+ struct drm_fb_helper_connector *fb_helper_connector;
+
+ if (!drm_fbdev_emulation)
+ return 0;
+
+ WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
+ if (fb_helper->connector_count + 1 > fb_helper->connector_info_alloc_count) {
+ temp = krealloc(fb_helper->connector_info, sizeof(struct drm_fb_helper_connector *) * (fb_helper->connector_count + 1), GFP_KERNEL);
+ if (!temp)
+ return -ENOMEM;
+
+ fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
+ fb_helper->connector_info = temp;
+ }
+
+ fb_helper_connector = kzalloc(sizeof(struct drm_fb_helper_connector), GFP_KERNEL);
+ if (!fb_helper_connector)
+ return -ENOMEM;
+
+ drm_connector_get(connector);
+ fb_helper_connector->connector = connector;
+ fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
+ return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
+
/**
* drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev
* emulation helper
@@ -162,36 +191,6 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
}
EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
-int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector)
-{
- struct drm_fb_helper_connector **temp;
- struct drm_fb_helper_connector *fb_helper_connector;
-
- if (!drm_fbdev_emulation)
- return 0;
-
- WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
- if (fb_helper->connector_count + 1 > fb_helper->connector_info_alloc_count) {
- temp = krealloc(fb_helper->connector_info, sizeof(struct drm_fb_helper_connector *) * (fb_helper->connector_count + 1), GFP_KERNEL);
- if (!temp)
- return -ENOMEM;
-
- fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
- fb_helper->connector_info = temp;
- }
-
-
- fb_helper_connector = kzalloc(sizeof(struct drm_fb_helper_connector), GFP_KERNEL);
- if (!fb_helper_connector)
- return -ENOMEM;
-
- drm_connector_get(connector);
- fb_helper_connector->connector = connector;
- fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
- return 0;
-}
-EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
-
int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
struct drm_connector *connector)
{
--
2.12.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 02/10] drm/fb-helper: Reshuffle code for subsequent patches
2017-03-21 8:13 ` [PATCH v3 02/10] drm/fb-helper: Reshuffle code for subsequent patches Thierry Reding
@ 2017-03-21 9:36 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 9:36 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, John Stultz, dri-devel
On Tue, Mar 21, 2017 at 09:13:50AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> An unlocked version of the drm_fb_helper_add_one_connector() function
> will be added in a subsequent patch. Reshuffle the code separately to
> make the diff more readable later on.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 59 ++++++++++++++++++++---------------------
> 1 file changed, 29 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 2be1cd949cec..875a49048222 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -109,6 +109,35 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
> for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
> i__ = 0; i__ < (fbh)->connector_count; i__++)
>
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector)
> +{
> + struct drm_fb_helper_connector **temp;
> + struct drm_fb_helper_connector *fb_helper_connector;
> +
> + if (!drm_fbdev_emulation)
> + return 0;
> +
> + WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
> + if (fb_helper->connector_count + 1 > fb_helper->connector_info_alloc_count) {
> + temp = krealloc(fb_helper->connector_info, sizeof(struct drm_fb_helper_connector *) * (fb_helper->connector_count + 1), GFP_KERNEL);
> + if (!temp)
> + return -ENOMEM;
> +
> + fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
> + fb_helper->connector_info = temp;
> + }
> +
> + fb_helper_connector = kzalloc(sizeof(struct drm_fb_helper_connector), GFP_KERNEL);
> + if (!fb_helper_connector)
> + return -ENOMEM;
> +
> + drm_connector_get(connector);
> + fb_helper_connector->connector = connector;
> + fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
> +
> /**
> * drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev
> * emulation helper
> @@ -162,36 +191,6 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> }
> EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
>
> -int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector)
> -{
> - struct drm_fb_helper_connector **temp;
> - struct drm_fb_helper_connector *fb_helper_connector;
> -
> - if (!drm_fbdev_emulation)
> - return 0;
> -
> - WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
> - if (fb_helper->connector_count + 1 > fb_helper->connector_info_alloc_count) {
> - temp = krealloc(fb_helper->connector_info, sizeof(struct drm_fb_helper_connector *) * (fb_helper->connector_count + 1), GFP_KERNEL);
> - if (!temp)
> - return -ENOMEM;
> -
> - fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
> - fb_helper->connector_info = temp;
> - }
> -
> -
> - fb_helper_connector = kzalloc(sizeof(struct drm_fb_helper_connector), GFP_KERNEL);
> - if (!fb_helper_connector)
> - return -ENOMEM;
> -
> - drm_connector_get(connector);
> - fb_helper_connector->connector = connector;
> - fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
> - return 0;
> -}
> -EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
> -
> int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> struct drm_connector *connector)
> {
> --
> 2.12.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 03/10] drm/fb-helper: Improve code readability
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
2017-03-21 8:13 ` [PATCH v3 01/10] drm/fb-helper: Cleanup checkpatch warnings Thierry Reding
2017-03-21 8:13 ` [PATCH v3 02/10] drm/fb-helper: Reshuffle code for subsequent patches Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 9:37 ` Daniel Vetter
2017-03-21 8:13 ` [PATCH v3 04/10] drm/fb-helper: Push down modeset lock into FB helpers Thierry Reding
` (7 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
Add a couple of temporary variables and use shorter names for existing
variables in drm_fb_helper_add_one_connector() for better readability.
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_fb_helper.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 875a49048222..17fd98fe4a1f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -109,31 +109,38 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
i__ = 0; i__ < (fbh)->connector_count; i__++)
-int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector)
+int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+ struct drm_connector *connector)
{
struct drm_fb_helper_connector **temp;
- struct drm_fb_helper_connector *fb_helper_connector;
+ struct drm_fb_helper_connector *conn;
+ unsigned int count;
if (!drm_fbdev_emulation)
return 0;
WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
- if (fb_helper->connector_count + 1 > fb_helper->connector_info_alloc_count) {
- temp = krealloc(fb_helper->connector_info, sizeof(struct drm_fb_helper_connector *) * (fb_helper->connector_count + 1), GFP_KERNEL);
+
+ count = fb_helper->connector_count + 1;
+
+ if (count > fb_helper->connector_info_alloc_count) {
+ size_t size = count * sizeof(conn);
+
+ temp = krealloc(fb_helper->connector_info, size, GFP_KERNEL);
if (!temp)
return -ENOMEM;
- fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
+ fb_helper->connector_info_alloc_count = count;
fb_helper->connector_info = temp;
}
- fb_helper_connector = kzalloc(sizeof(struct drm_fb_helper_connector), GFP_KERNEL);
- if (!fb_helper_connector)
+ conn = kzalloc(sizeof(*conn), GFP_KERNEL);
+ if (!conn)
return -ENOMEM;
drm_connector_get(connector);
- fb_helper_connector->connector = connector;
- fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
+ conn->connector = connector;
+ fb_helper->connector_info[fb_helper->connector_count++] = conn;
return 0;
}
EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
--
2.12.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 03/10] drm/fb-helper: Improve code readability
2017-03-21 8:13 ` [PATCH v3 03/10] drm/fb-helper: Improve code readability Thierry Reding
@ 2017-03-21 9:37 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 9:37 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Tue, Mar 21, 2017 at 09:13:51AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Add a couple of temporary variables and use shorter names for existing
> variables in drm_fb_helper_add_one_connector() for better readability.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 875a49048222..17fd98fe4a1f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -109,31 +109,38 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
> for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
> i__ = 0; i__ < (fbh)->connector_count; i__++)
>
> -int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector)
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> {
> struct drm_fb_helper_connector **temp;
> - struct drm_fb_helper_connector *fb_helper_connector;
> + struct drm_fb_helper_connector *conn;
I'd go with fb_conn here. Still short, but avoids confusion with
drm_connector, which is what we usually call conn. Either way:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> + unsigned int count;
>
> if (!drm_fbdev_emulation)
> return 0;
>
> WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
> - if (fb_helper->connector_count + 1 > fb_helper->connector_info_alloc_count) {
> - temp = krealloc(fb_helper->connector_info, sizeof(struct drm_fb_helper_connector *) * (fb_helper->connector_count + 1), GFP_KERNEL);
> +
> + count = fb_helper->connector_count + 1;
> +
> + if (count > fb_helper->connector_info_alloc_count) {
> + size_t size = count * sizeof(conn);
> +
> + temp = krealloc(fb_helper->connector_info, size, GFP_KERNEL);
> if (!temp)
> return -ENOMEM;
>
> - fb_helper->connector_info_alloc_count = fb_helper->connector_count + 1;
> + fb_helper->connector_info_alloc_count = count;
> fb_helper->connector_info = temp;
> }
>
> - fb_helper_connector = kzalloc(sizeof(struct drm_fb_helper_connector), GFP_KERNEL);
> - if (!fb_helper_connector)
> + conn = kzalloc(sizeof(*conn), GFP_KERNEL);
> + if (!conn)
> return -ENOMEM;
>
> drm_connector_get(connector);
> - fb_helper_connector->connector = connector;
> - fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
> + conn->connector = connector;
> + fb_helper->connector_info[fb_helper->connector_count++] = conn;
> return 0;
> }
> EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
> --
> 2.12.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 04/10] drm/fb-helper: Push down modeset lock into FB helpers
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (2 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 03/10] drm/fb-helper: Improve code readability Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 9:38 ` Daniel Vetter
2017-03-21 8:13 ` [PATCH v3 05/10] drm/fb-helper: Add top-level lock Thierry Reding
` (6 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: John Stultz, intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
Move the modeset locking from drivers into FB helpers.
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_fb_helper.c | 39 ++++++++++++++++++++++++++++------
drivers/gpu/drm/i915/intel_dp_mst.c | 3 ---
drivers/gpu/drm/radeon/radeon_dp_mst.c | 7 ------
3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 17fd98fe4a1f..6af80afc4f40 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -109,8 +109,8 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
i__ = 0; i__ < (fbh)->connector_count; i__++)
-int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
- struct drm_connector *connector)
+static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+ struct drm_connector *connector)
{
struct drm_fb_helper_connector **temp;
struct drm_fb_helper_connector *conn;
@@ -143,6 +143,20 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
fb_helper->connector_info[fb_helper->connector_count++] = conn;
return 0;
}
+
+int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+ struct drm_connector *connector)
+{
+ int err;
+
+ mutex_lock(&fb_helper->dev->mode_config.mutex);
+
+ err = __drm_fb_helper_add_one_connector(fb_helper, connector);
+
+ mutex_unlock(&fb_helper->dev->mode_config.mutex);
+
+ return err;
+}
EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
/**
@@ -172,8 +186,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
mutex_lock(&dev->mode_config.mutex);
drm_connector_list_iter_begin(dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
- ret = drm_fb_helper_add_one_connector(fb_helper, connector);
-
+ ret = __drm_fb_helper_add_one_connector(fb_helper, connector);
if (ret)
goto fail;
}
@@ -198,8 +211,8 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
}
EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
-int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
- struct drm_connector *connector)
+static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
+ struct drm_connector *connector)
{
struct drm_fb_helper_connector *fb_helper_connector;
int i, j;
@@ -227,6 +240,20 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
return 0;
}
+
+int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
+ struct drm_connector *connector)
+{
+ int err;
+
+ mutex_lock(&fb_helper->dev->mode_config.mutex);
+
+ err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
+
+ mutex_unlock(&fb_helper->dev->mode_config.mutex);
+
+ return err;
+}
EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 094cbdcbcd6d..8336c5c6f45d 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -484,15 +484,12 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
struct drm_connector *connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
- struct drm_device *dev = connector->dev;
drm_connector_unregister(connector);
/* need to nuke the connector */
- drm_modeset_lock_all(dev);
intel_connector_remove_from_fbdev(intel_connector);
intel_connector->mst_port = NULL;
- drm_modeset_unlock_all(dev);
drm_connector_unreference(&intel_connector->base);
DRM_DEBUG_KMS("\n");
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index 7d5ada3980dc..b1855cf0ae7a 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -300,9 +300,7 @@ static void radeon_dp_register_mst_connector(struct drm_connector *connector)
struct drm_device *dev = connector->dev;
struct radeon_device *rdev = dev->dev_private;
- drm_modeset_lock_all(dev);
radeon_fb_add_connector(rdev, connector);
- drm_modeset_unlock_all(dev);
drm_connector_register(connector);
}
@@ -315,13 +313,8 @@ static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
struct radeon_device *rdev = dev->dev_private;
drm_connector_unregister(connector);
- /* need to nuke the connector */
- drm_modeset_lock_all(dev);
- /* dpms off */
radeon_fb_remove_connector(rdev, connector);
-
drm_connector_cleanup(connector);
- drm_modeset_unlock_all(dev);
kfree(connector);
DRM_DEBUG_KMS("\n");
--
2.12.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 04/10] drm/fb-helper: Push down modeset lock into FB helpers
2017-03-21 8:13 ` [PATCH v3 04/10] drm/fb-helper: Push down modeset lock into FB helpers Thierry Reding
@ 2017-03-21 9:38 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 9:38 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, John Stultz, dri-devel
On Tue, Mar 21, 2017 at 09:13:52AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Move the modeset locking from drivers into FB helpers.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 39 ++++++++++++++++++++++++++++------
> drivers/gpu/drm/i915/intel_dp_mst.c | 3 ---
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 7 ------
> 3 files changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 17fd98fe4a1f..6af80afc4f40 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -109,8 +109,8 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
> for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
> i__ = 0; i__ < (fbh)->connector_count; i__++)
>
> -int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> - struct drm_connector *connector)
> +static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> {
> struct drm_fb_helper_connector **temp;
> struct drm_fb_helper_connector *conn;
> @@ -143,6 +143,20 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> fb_helper->connector_info[fb_helper->connector_count++] = conn;
> return 0;
> }
> +
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> +{
> + int err;
> +
> + mutex_lock(&fb_helper->dev->mode_config.mutex);
> +
> + err = __drm_fb_helper_add_one_connector(fb_helper, connector);
> +
> + mutex_unlock(&fb_helper->dev->mode_config.mutex);
> +
> + return err;
> +}
> EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
>
> /**
> @@ -172,8 +186,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> mutex_lock(&dev->mode_config.mutex);
> drm_connector_list_iter_begin(dev, &conn_iter);
> drm_for_each_connector_iter(connector, &conn_iter) {
> - ret = drm_fb_helper_add_one_connector(fb_helper, connector);
> -
> + ret = __drm_fb_helper_add_one_connector(fb_helper, connector);
> if (ret)
> goto fail;
> }
> @@ -198,8 +211,8 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> }
> EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
>
> -int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> - struct drm_connector *connector)
> +static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> {
> struct drm_fb_helper_connector *fb_helper_connector;
> int i, j;
> @@ -227,6 +240,20 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>
> return 0;
> }
> +
> +int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> +{
> + int err;
> +
> + mutex_lock(&fb_helper->dev->mode_config.mutex);
> +
> + err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
> +
> + mutex_unlock(&fb_helper->dev->mode_config.mutex);
> +
> + return err;
> +}
> EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>
> static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 094cbdcbcd6d..8336c5c6f45d 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -484,15 +484,12 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_connector *connector)
> {
> struct intel_connector *intel_connector = to_intel_connector(connector);
> - struct drm_device *dev = connector->dev;
>
> drm_connector_unregister(connector);
>
> /* need to nuke the connector */
> - drm_modeset_lock_all(dev);
> intel_connector_remove_from_fbdev(intel_connector);
> intel_connector->mst_port = NULL;
> - drm_modeset_unlock_all(dev);
>
> drm_connector_unreference(&intel_connector->base);
> DRM_DEBUG_KMS("\n");
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index 7d5ada3980dc..b1855cf0ae7a 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -300,9 +300,7 @@ static void radeon_dp_register_mst_connector(struct drm_connector *connector)
> struct drm_device *dev = connector->dev;
> struct radeon_device *rdev = dev->dev_private;
>
> - drm_modeset_lock_all(dev);
> radeon_fb_add_connector(rdev, connector);
> - drm_modeset_unlock_all(dev);
>
> drm_connector_register(connector);
> }
> @@ -315,13 +313,8 @@ static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
> struct radeon_device *rdev = dev->dev_private;
>
> drm_connector_unregister(connector);
> - /* need to nuke the connector */
> - drm_modeset_lock_all(dev);
> - /* dpms off */
> radeon_fb_remove_connector(rdev, connector);
> -
> drm_connector_cleanup(connector);
> - drm_modeset_unlock_all(dev);
>
> kfree(connector);
> DRM_DEBUG_KMS("\n");
> --
> 2.12.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 05/10] drm/fb-helper: Add top-level lock
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (3 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 04/10] drm/fb-helper: Push down modeset lock into FB helpers Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 10:00 ` Daniel Vetter
2017-03-21 8:13 ` [PATCH v3 06/10] drm/fb-helper: Support deferred setup Thierry Reding
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
Introduce a new top-level lock for the FB helper code. This will allow
better locking granularity and avoid the need to abuse modeset locking
for this purpose instead.
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_fb_helper.c | 27 ++++++++++++++++++++++++++-
include/drm/drm_fb_helper.h | 7 +++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 6af80afc4f40..9060adcf7cf8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -119,6 +119,7 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
if (!drm_fbdev_emulation)
return 0;
+ WARN_ON(!mutex_is_locked(&fb_helper->lock));
WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
count = fb_helper->connector_count + 1;
@@ -141,6 +142,7 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
drm_connector_get(connector);
conn->connector = connector;
fb_helper->connector_info[fb_helper->connector_count++] = conn;
+
return 0;
}
@@ -149,11 +151,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
{
int err;
+ mutex_lock(&fb_helper->lock);
mutex_lock(&fb_helper->dev->mode_config.mutex);
err = __drm_fb_helper_add_one_connector(fb_helper, connector);
mutex_unlock(&fb_helper->dev->mode_config.mutex);
+ mutex_unlock(&fb_helper->lock);
return err;
}
@@ -183,6 +187,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
if (!drm_fbdev_emulation)
return 0;
+ mutex_lock(&fb_helper->lock);
mutex_lock(&dev->mode_config.mutex);
drm_connector_list_iter_begin(dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
@@ -206,6 +211,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
out:
drm_connector_list_iter_end(&conn_iter);
mutex_unlock(&dev->mode_config.mutex);
+ mutex_unlock(&fb_helper->lock);
return ret;
}
@@ -220,6 +226,7 @@ static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
if (!drm_fbdev_emulation)
return 0;
+ WARN_ON(!mutex_is_locked(&fb_helper->lock));
WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
for (i = 0; i < fb_helper->connector_count; i++) {
@@ -246,11 +253,13 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
{
int err;
+ mutex_lock(&fb_helper->lock);
mutex_lock(&fb_helper->dev->mode_config.mutex);
err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
mutex_unlock(&fb_helper->dev->mode_config.mutex);
+ mutex_unlock(&fb_helper->lock);
return err;
}
@@ -502,16 +511,21 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
if (!drm_fbdev_emulation)
return -ENODEV;
+ mutex_lock(&fb_helper->lock);
drm_modeset_lock_all(dev);
+
ret = restore_fbdev_mode(fb_helper);
do_delayed = fb_helper->delayed_hotplug;
if (do_delayed)
fb_helper->delayed_hotplug = false;
+
drm_modeset_unlock_all(dev);
+ mutex_unlock(&fb_helper->lock);
if (do_delayed)
drm_fb_helper_hotplug_event(fb_helper);
+
return ret;
}
EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
@@ -747,6 +761,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work);
helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
+ mutex_init(&helper->lock);
helper->funcs = funcs;
helper->dev = dev;
}
@@ -912,6 +927,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
}
mutex_unlock(&kernel_fb_helper_lock);
+ mutex_destroy(&fb_helper->lock);
drm_fb_helper_crtc_free(fb_helper);
}
@@ -2421,25 +2437,34 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
{
struct drm_device *dev = fb_helper->dev;
+ int err = 0;
if (!drm_fbdev_emulation)
return 0;
+ mutex_lock(&fb_helper->lock);
mutex_lock(&dev->mode_config.mutex);
+
if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
fb_helper->delayed_hotplug = true;
mutex_unlock(&dev->mode_config.mutex);
- return 0;
+ goto unlock;
}
+
DRM_DEBUG_KMS("\n");
drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
mutex_unlock(&dev->mode_config.mutex);
+ mutex_unlock(&fb_helper->lock);
drm_fb_helper_set_par(fb_helper->fbdev);
return 0;
+
+unlock:
+ mutex_unlock(&fb_helper->lock);
+ return err;
}
EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 119e5e4609c7..33771d0c44e3 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -201,6 +201,13 @@ struct drm_fb_helper {
struct work_struct resume_work;
/**
+ * @lock:
+ *
+ * Top-level FB helper lock.
+ */
+ struct mutex lock;
+
+ /**
* @kernel_fb_list:
*
* Entry on the global kernel_fb_helper_list, used for kgdb entry/exit.
--
2.12.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/10] drm/fb-helper: Add top-level lock
2017-03-21 8:13 ` [PATCH v3 05/10] drm/fb-helper: Add top-level lock Thierry Reding
@ 2017-03-21 10:00 ` Daniel Vetter
2017-03-21 10:15 ` Daniel Vetter
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 10:00 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Tue, Mar 21, 2017 at 09:13:53AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Introduce a new top-level lock for the FB helper code. This will allow
> better locking granularity and avoid the need to abuse modeset locking
> for this purpose instead.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Some suggestions below, since I think this state of locking implemented
here is a bit a too awkwared intermediate thing.
> ---
> drivers/gpu/drm/drm_fb_helper.c | 27 ++++++++++++++++++++++++++-
> include/drm/drm_fb_helper.h | 7 +++++++
> 2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 6af80afc4f40..9060adcf7cf8 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -119,6 +119,7 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> if (!drm_fbdev_emulation)
> return 0;
>
> + WARN_ON(!mutex_is_locked(&fb_helper->lock));
> WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
mode_config.mutex isn't needed anymore here (with the recommendations at
the bottom), please remove.
>
> count = fb_helper->connector_count + 1;
> @@ -141,6 +142,7 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> drm_connector_get(connector);
> conn->connector = connector;
> fb_helper->connector_info[fb_helper->connector_count++] = conn;
> +
> return 0;
> }
>
> @@ -149,11 +151,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> {
> int err;
>
> + mutex_lock(&fb_helper->lock);
> mutex_lock(&fb_helper->dev->mode_config.mutex);
Same.
>
> err = __drm_fb_helper_add_one_connector(fb_helper, connector);
>
> mutex_unlock(&fb_helper->dev->mode_config.mutex);
> + mutex_unlock(&fb_helper->lock);
>
> return err;
> }
> @@ -183,6 +187,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return 0;
>
> + mutex_lock(&fb_helper->lock);
> mutex_lock(&dev->mode_config.mutex);
Same.
> drm_connector_list_iter_begin(dev, &conn_iter);
> drm_for_each_connector_iter(connector, &conn_iter) {
> @@ -206,6 +211,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> out:
> drm_connector_list_iter_end(&conn_iter);
> mutex_unlock(&dev->mode_config.mutex);
> + mutex_unlock(&fb_helper->lock);
>
> return ret;
> }
> @@ -220,6 +226,7 @@ static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> if (!drm_fbdev_emulation)
> return 0;
>
> + WARN_ON(!mutex_is_locked(&fb_helper->lock));
> WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
Same.
>
> for (i = 0; i < fb_helper->connector_count; i++) {
> @@ -246,11 +253,13 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> {
> int err;
>
> + mutex_lock(&fb_helper->lock);
> mutex_lock(&fb_helper->dev->mode_config.mutex);
>
> err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
>
> mutex_unlock(&fb_helper->dev->mode_config.mutex);
> + mutex_unlock(&fb_helper->lock);
>
> return err;
> }
> @@ -502,16 +511,21 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return -ENODEV;
>
> + mutex_lock(&fb_helper->lock);
> drm_modeset_lock_all(dev);
> +
> ret = restore_fbdev_mode(fb_helper);
>
> do_delayed = fb_helper->delayed_hotplug;
> if (do_delayed)
> fb_helper->delayed_hotplug = false;
> +
> drm_modeset_unlock_all(dev);
> + mutex_unlock(&fb_helper->lock);
>
> if (do_delayed)
> drm_fb_helper_hotplug_event(fb_helper);
> +
> return ret;
> }
> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
> @@ -747,6 +761,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
> INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
> INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work);
> helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
> + mutex_init(&helper->lock);
> helper->funcs = funcs;
> helper->dev = dev;
> }
> @@ -912,6 +927,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
> }
> mutex_unlock(&kernel_fb_helper_lock);
>
> + mutex_destroy(&fb_helper->lock);
> drm_fb_helper_crtc_free(fb_helper);
>
> }
> @@ -2421,25 +2437,34 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> + int err = 0;
>
> if (!drm_fbdev_emulation)
> return 0;
>
> + mutex_lock(&fb_helper->lock);
> mutex_lock(&dev->mode_config.mutex);
> +
> if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
> fb_helper->delayed_hotplug = true;
> mutex_unlock(&dev->mode_config.mutex);
> - return 0;
> + goto unlock;
> }
> +
> DRM_DEBUG_KMS("\n");
>
> drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
>
> mutex_unlock(&dev->mode_config.mutex);
> + mutex_unlock(&fb_helper->lock);
>
> drm_fb_helper_set_par(fb_helper->fbdev);
>
> return 0;
> +
> +unlock:
> + mutex_unlock(&fb_helper->lock);
> + return err;
> }
> EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 119e5e4609c7..33771d0c44e3 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -201,6 +201,13 @@ struct drm_fb_helper {
> struct work_struct resume_work;
>
> /**
> + * @lock:
> + *
> + * Top-level FB helper lock.
Bit more detail would be good here.
"This protects all the internal data structures and lists, like
@connector_info or @crtc_info."
> + */
> + struct mutex lock;
> +
> + /**
> * @kernel_fb_list:
> *
> * Entry on the global kernel_fb_helper_list, used for kgdb entry/exit.
So I think you're missing to protect almost all the fbdev paths.
Essentially anything looking at crtc_info or connector_info must hold the
new lock, which means all the places that currently call
drm_modeset_lock_all() need to grab this new lock too.
Best way to make sure you catch them all is to also update the lockdep
assert in drm_fb_helper_for_each_connector().
There's still a massive mess going with locking here, especially around
the probe code and drm_fb_helper_is_bound, but that's a different thing
and better left for cleanup later on.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/10] drm/fb-helper: Add top-level lock
2017-03-21 10:00 ` Daniel Vetter
@ 2017-03-21 10:15 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 10:15 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Tue, Mar 21, 2017 at 11:00:23AM +0100, Daniel Vetter wrote:
> On Tue, Mar 21, 2017 at 09:13:53AM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Introduce a new top-level lock for the FB helper code. This will allow
> > better locking granularity and avoid the need to abuse modeset locking
> > for this purpose instead.
> >
> > Tested-by: John Stultz <john.stultz@linaro.org>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
>
> Some suggestions below, since I think this state of locking implemented
> here is a bit a too awkwared intermediate thing.
To clarify: After having reviewed the next patch I think this here is
fine. So gets an Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
as-is. I just think we want a notch more here.
But if you're burried too much we can do that as a follow-up too, but in
that case please add a FIXME to e.g. the @lock kernel-doc:
"FIXME: fbdev emulation locking is a mess, long term we want to protect
all helper internal state with this lock, and reduce core KMS locking as
much as possible."
Your choice :-)
Cheers, Daniel
>
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 27 ++++++++++++++++++++++++++-
> > include/drm/drm_fb_helper.h | 7 +++++++
> > 2 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 6af80afc4f40..9060adcf7cf8 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -119,6 +119,7 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> > if (!drm_fbdev_emulation)
> > return 0;
> >
> > + WARN_ON(!mutex_is_locked(&fb_helper->lock));
> > WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
>
> mode_config.mutex isn't needed anymore here (with the recommendations at
> the bottom), please remove.
> >
> > count = fb_helper->connector_count + 1;
> > @@ -141,6 +142,7 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> > drm_connector_get(connector);
> > conn->connector = connector;
> > fb_helper->connector_info[fb_helper->connector_count++] = conn;
> > +
> > return 0;
> > }
> >
> > @@ -149,11 +151,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> > {
> > int err;
> >
> > + mutex_lock(&fb_helper->lock);
> > mutex_lock(&fb_helper->dev->mode_config.mutex);
>
> Same.
>
> >
> > err = __drm_fb_helper_add_one_connector(fb_helper, connector);
> >
> > mutex_unlock(&fb_helper->dev->mode_config.mutex);
> > + mutex_unlock(&fb_helper->lock);
> >
> > return err;
> > }
> > @@ -183,6 +187,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> > if (!drm_fbdev_emulation)
> > return 0;
> >
> > + mutex_lock(&fb_helper->lock);
> > mutex_lock(&dev->mode_config.mutex);
>
> Same.
>
> > drm_connector_list_iter_begin(dev, &conn_iter);
> > drm_for_each_connector_iter(connector, &conn_iter) {
> > @@ -206,6 +211,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> > out:
> > drm_connector_list_iter_end(&conn_iter);
> > mutex_unlock(&dev->mode_config.mutex);
> > + mutex_unlock(&fb_helper->lock);
> >
> > return ret;
> > }
> > @@ -220,6 +226,7 @@ static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> > if (!drm_fbdev_emulation)
> > return 0;
> >
> > + WARN_ON(!mutex_is_locked(&fb_helper->lock));
> > WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
>
> Same.
>
> >
> > for (i = 0; i < fb_helper->connector_count; i++) {
> > @@ -246,11 +253,13 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> > {
> > int err;
> >
> > + mutex_lock(&fb_helper->lock);
> > mutex_lock(&fb_helper->dev->mode_config.mutex);
> >
> > err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
> >
> > mutex_unlock(&fb_helper->dev->mode_config.mutex);
> > + mutex_unlock(&fb_helper->lock);
> >
> > return err;
> > }
> > @@ -502,16 +511,21 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> > if (!drm_fbdev_emulation)
> > return -ENODEV;
> >
> > + mutex_lock(&fb_helper->lock);
> > drm_modeset_lock_all(dev);
> > +
> > ret = restore_fbdev_mode(fb_helper);
> >
> > do_delayed = fb_helper->delayed_hotplug;
> > if (do_delayed)
> > fb_helper->delayed_hotplug = false;
> > +
> > drm_modeset_unlock_all(dev);
> > + mutex_unlock(&fb_helper->lock);
> >
> > if (do_delayed)
> > drm_fb_helper_hotplug_event(fb_helper);
> > +
> > return ret;
> > }
> > EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
> > @@ -747,6 +761,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
> > INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
> > INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work);
> > helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
> > + mutex_init(&helper->lock);
> > helper->funcs = funcs;
> > helper->dev = dev;
> > }
> > @@ -912,6 +927,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
> > }
> > mutex_unlock(&kernel_fb_helper_lock);
> >
> > + mutex_destroy(&fb_helper->lock);
> > drm_fb_helper_crtc_free(fb_helper);
> >
> > }
> > @@ -2421,25 +2437,34 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> > int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> > {
> > struct drm_device *dev = fb_helper->dev;
> > + int err = 0;
> >
> > if (!drm_fbdev_emulation)
> > return 0;
> >
> > + mutex_lock(&fb_helper->lock);
> > mutex_lock(&dev->mode_config.mutex);
> > +
> > if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
> > fb_helper->delayed_hotplug = true;
> > mutex_unlock(&dev->mode_config.mutex);
> > - return 0;
> > + goto unlock;
> > }
> > +
> > DRM_DEBUG_KMS("\n");
> >
> > drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
> >
> > mutex_unlock(&dev->mode_config.mutex);
> > + mutex_unlock(&fb_helper->lock);
> >
> > drm_fb_helper_set_par(fb_helper->fbdev);
> >
> > return 0;
> > +
> > +unlock:
> > + mutex_unlock(&fb_helper->lock);
> > + return err;
> > }
> > EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
> >
> > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> > index 119e5e4609c7..33771d0c44e3 100644
> > --- a/include/drm/drm_fb_helper.h
> > +++ b/include/drm/drm_fb_helper.h
> > @@ -201,6 +201,13 @@ struct drm_fb_helper {
> > struct work_struct resume_work;
> >
> > /**
> > + * @lock:
> > + *
> > + * Top-level FB helper lock.
>
> Bit more detail would be good here.
>
> "This protects all the internal data structures and lists, like
> @connector_info or @crtc_info."
>
> > + */
> > + struct mutex lock;
> > +
> > + /**
> > * @kernel_fb_list:
> > *
> > * Entry on the global kernel_fb_helper_list, used for kgdb entry/exit.
>
> So I think you're missing to protect almost all the fbdev paths.
> Essentially anything looking at crtc_info or connector_info must hold the
> new lock, which means all the places that currently call
> drm_modeset_lock_all() need to grab this new lock too.
>
> Best way to make sure you catch them all is to also update the lockdep
> assert in drm_fb_helper_for_each_connector().
>
> There's still a massive mess going with locking here, especially around
> the probe code and drm_fb_helper_is_bound, but that's a different thing
> and better left for cleanup later on.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 06/10] drm/fb-helper: Support deferred setup
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (4 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 05/10] drm/fb-helper: Add top-level lock Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 10:10 ` Daniel Vetter
2017-04-03 8:48 ` Daniel Vetter
2017-03-21 8:13 ` [PATCH v3 07/10] drm/exynos: Remove custom FB helper " Thierry Reding
` (4 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
FB helper code falls back to a 1024x768 mode if no outputs are connected
or don't report back any modes upon initialization. This can be annoying
because outputs that are added to FB helper later on can't be used with
FB helper if they don't support a matching mode.
The fallback is in place because VGA connectors can happen to report an
unknown connection status even when they are in fact connected.
Some drivers have custom solutions in place to defer FB helper setup
until at least one output is connected. But the logic behind these
solutions is always the same and there is nothing driver-specific about
it, so a better alterative is to fix the FB helper core and add support
for all drivers automatically.
This patch adds support for deferred FB helper setup. It checks all the
connectors for their connection status, and if all of them report to be
disconnected marks the FB helper as needing deferred setup. Whet setup
is deferred, the FB helper core will automatically retry setup after a
hotplug event, and it will keep trying until it succeeds.
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_fb_helper.c | 60 +++++++++++++++++++++++++++++++++++++----
include/drm/drm_fb_helper.h | 21 +++++++++++++++
2 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 9060adcf7cf8..d4a2c97d8b02 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -511,6 +511,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
if (!drm_fbdev_emulation)
return -ENODEV;
+ if (fb_helper->deferred_setup)
+ return 0;
+
mutex_lock(&fb_helper->lock);
drm_modeset_lock_all(dev);
@@ -1597,6 +1600,23 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
}
EXPORT_SYMBOL(drm_fb_helper_pan_display);
+static bool drm_fb_helper_maybe_connected(struct drm_fb_helper *helper)
+{
+ bool connected = false;
+ unsigned int i;
+
+ for (i = 0; i < helper->connector_count; i++) {
+ struct drm_fb_helper_connector *fb = helper->connector_info[i];
+
+ if (fb->connector->status != connector_status_disconnected) {
+ connected = true;
+ break;
+ }
+ }
+
+ return connected;
+}
+
/*
* Allocates the backing storage and sets up the fbdev info structure through
* the ->fb_probe callback and then registers the fbdev and sets up the panic
@@ -2254,8 +2274,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
int i;
DRM_DEBUG_KMS("\n");
- if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
- DRM_DEBUG_KMS("No connectors reported connected with modes\n");
/* prevent concurrent modification of connector_count by hotplug */
lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
@@ -2378,6 +2396,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
{
struct drm_device *dev = fb_helper->dev;
+ unsigned int width, height;
struct fb_info *info;
int ret;
@@ -2385,14 +2404,34 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
return 0;
mutex_lock(&dev->mode_config.mutex);
- drm_setup_crtcs(fb_helper,
- dev->mode_config.max_width,
- dev->mode_config.max_height);
+
+ width = dev->mode_config.max_width;
+ height = dev->mode_config.max_height;
+
+ if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
+ DRM_DEBUG_KMS("No connectors reported connected with modes\n");
+
+ /*
+ * If everything's disconnected, there's no use in attempting to set
+ * up fbdev.
+ */
+ if (!drm_fb_helper_maybe_connected(fb_helper)) {
+ DRM_INFO("No outputs connected, deferring setup\n");
+ fb_helper->preferred_bpp = bpp_sel;
+ fb_helper->deferred_setup = true;
+ mutex_unlock(&dev->mode_config.mutex);
+ return 0;
+ }
+
+ drm_setup_crtcs(fb_helper, width, height);
+
ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
mutex_unlock(&dev->mode_config.mutex);
if (ret)
return ret;
+ fb_helper->deferred_setup = false;
+
info = fb_helper->fbdev;
info->var.pixclock = 0;
ret = register_framebuffer(info);
@@ -2437,11 +2476,16 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
{
struct drm_device *dev = fb_helper->dev;
+ unsigned int width, height;
int err = 0;
if (!drm_fbdev_emulation)
return 0;
+ if (fb_helper->deferred_setup)
+ return drm_fb_helper_initial_config(fb_helper,
+ fb_helper->preferred_bpp);
+
mutex_lock(&fb_helper->lock);
mutex_lock(&dev->mode_config.mutex);
@@ -2453,6 +2497,12 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
DRM_DEBUG_KMS("\n");
+ width = dev->mode_config.max_width;
+ height = dev->mode_config.max_height;
+
+ if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
+ DRM_DEBUG_KMS("No connectors reported connected with modes\n");
+
drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
mutex_unlock(&dev->mode_config.mutex);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 33771d0c44e3..41c3587751f8 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -222,6 +222,27 @@ struct drm_fb_helper {
* needs to be reprobe when fbdev is in control again.
*/
bool delayed_hotplug;
+
+ /**
+ * @deferred_setup:
+ *
+ * If no outputs are connected (disconnected or unknown) the FB helper
+ * code will defer setup until at least one of the outputs shows up.
+ * This field keeps track of the status so that setup can be retried
+ * at every hotplug event until it succeeds eventually.
+ */
+ bool deferred_setup;
+
+ /**
+ * @preferred_bpp:
+ *
+ * Temporary storage for the driver's preferred BPP setting passed to
+ * FB helper initialization. This needs to be tracked so that deferred
+ * FB helper setup can pass this on.
+ *
+ * See also: @deferred_setup
+ */
+ int preferred_bpp;
};
/**
--
2.12.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 06/10] drm/fb-helper: Support deferred setup
2017-03-21 8:13 ` [PATCH v3 06/10] drm/fb-helper: Support deferred setup Thierry Reding
@ 2017-03-21 10:10 ` Daniel Vetter
2017-03-22 21:06 ` Thierry Reding
2017-04-03 8:48 ` Daniel Vetter
1 sibling, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 10:10 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Tue, Mar 21, 2017 at 09:13:54AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> FB helper code falls back to a 1024x768 mode if no outputs are connected
> or don't report back any modes upon initialization. This can be annoying
> because outputs that are added to FB helper later on can't be used with
> FB helper if they don't support a matching mode.
>
> The fallback is in place because VGA connectors can happen to report an
> unknown connection status even when they are in fact connected.
>
> Some drivers have custom solutions in place to defer FB helper setup
> until at least one output is connected. But the logic behind these
> solutions is always the same and there is nothing driver-specific about
> it, so a better alterative is to fix the FB helper core and add support
> for all drivers automatically.
>
> This patch adds support for deferred FB helper setup. It checks all the
> connectors for their connection status, and if all of them report to be
> disconnected marks the FB helper as needing deferred setup. Whet setup
> is deferred, the FB helper core will automatically retry setup after a
> hotplug event, and it will keep trying until it succeeds.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Tiny nitpicks below, with those addressed:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 60 +++++++++++++++++++++++++++++++++++++----
> include/drm/drm_fb_helper.h | 21 +++++++++++++++
> 2 files changed, 76 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9060adcf7cf8..d4a2c97d8b02 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -511,6 +511,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return -ENODEV;
>
> + if (fb_helper->deferred_setup)
> + return 0;
READ_ONCE or put it under the ->lock protection. Just for clarity.
> +
> mutex_lock(&fb_helper->lock);
> drm_modeset_lock_all(dev);
>
> @@ -1597,6 +1600,23 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
>
> +static bool drm_fb_helper_maybe_connected(struct drm_fb_helper *helper)
> +{
> + bool connected = false;
> + unsigned int i;
> +
> + for (i = 0; i < helper->connector_count; i++) {
> + struct drm_fb_helper_connector *fb = helper->connector_info[i];
> +
> + if (fb->connector->status != connector_status_disconnected) {
> + connected = true;
> + break;
> + }
> + }
> +
> + return connected;
> +}
> +
> /*
> * Allocates the backing storage and sets up the fbdev info structure through
> * the ->fb_probe callback and then registers the fbdev and sets up the panic
> @@ -2254,8 +2274,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> int i;
>
> DRM_DEBUG_KMS("\n");
> - if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> - DRM_DEBUG_KMS("No connectors reported connected with modes\n");
>
> /* prevent concurrent modification of connector_count by hotplug */
> lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
> @@ -2378,6 +2396,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
> {
> struct drm_device *dev = fb_helper->dev;
> + unsigned int width, height;
> struct fb_info *info;
> int ret;
>
> @@ -2385,14 +2404,34 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
> return 0;
>
> mutex_lock(&dev->mode_config.mutex);
> - drm_setup_crtcs(fb_helper,
> - dev->mode_config.max_width,
> - dev->mode_config.max_height);
> +
> + width = dev->mode_config.max_width;
> + height = dev->mode_config.max_height;
> +
> + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> + DRM_DEBUG_KMS("No connectors reported connected with modes\n");
> +
> + /*
> + * If everything's disconnected, there's no use in attempting to set
> + * up fbdev.
> + */
> + if (!drm_fb_helper_maybe_connected(fb_helper)) {
> + DRM_INFO("No outputs connected, deferring setup\n");
> + fb_helper->preferred_bpp = bpp_sel;
> + fb_helper->deferred_setup = true;
> + mutex_unlock(&dev->mode_config.mutex);
> + return 0;
> + }
> +
> + drm_setup_crtcs(fb_helper, width, height);
> +
> ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> mutex_unlock(&dev->mode_config.mutex);
> if (ret)
> return ret;
>
> + fb_helper->deferred_setup = false;
> +
> info = fb_helper->fbdev;
> info->var.pixclock = 0;
> ret = register_framebuffer(info);
> @@ -2437,11 +2476,16 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> + unsigned int width, height;
> int err = 0;
>
> if (!drm_fbdev_emulation)
> return 0;
>
> + if (fb_helper->deferred_setup)
> + return drm_fb_helper_initial_config(fb_helper,
> + fb_helper->preferred_bpp);
I think this must be moved under the protection of ->lock, you might race
otherwise (e.g. hpd vs. userspace forcing a re-probe, both noticing the
change).
> +
> mutex_lock(&fb_helper->lock);
> mutex_lock(&dev->mode_config.mutex);
>
> @@ -2453,6 +2497,12 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>
> DRM_DEBUG_KMS("\n");
>
> + width = dev->mode_config.max_width;
> + height = dev->mode_config.max_height;
> +
> + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> + DRM_DEBUG_KMS("No connectors reported connected with modes\n");
> +
> drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
>
> mutex_unlock(&dev->mode_config.mutex);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 33771d0c44e3..41c3587751f8 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -222,6 +222,27 @@ struct drm_fb_helper {
> * needs to be reprobe when fbdev is in control again.
> */
> bool delayed_hotplug;
> +
> + /**
> + * @deferred_setup:
> + *
> + * If no outputs are connected (disconnected or unknown) the FB helper
> + * code will defer setup until at least one of the outputs shows up.
> + * This field keeps track of the status so that setup can be retried
> + * at every hotplug event until it succeeds eventually.
Please add:
"Protected by @lock."
> + */
> + bool deferred_setup;
> +
> + /**
> + * @preferred_bpp:
> + *
> + * Temporary storage for the driver's preferred BPP setting passed to
> + * FB helper initialization. This needs to be tracked so that deferred
> + * FB helper setup can pass this on.
> + *
> + * See also: @deferred_setup
> + */
> + int preferred_bpp;
> };
>
> /**
> --
> 2.12.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 06/10] drm/fb-helper: Support deferred setup
2017-03-21 10:10 ` Daniel Vetter
@ 2017-03-22 21:06 ` Thierry Reding
2017-03-22 21:08 ` Daniel Vetter
0 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-22 21:06 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, intel-gfx, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 1119 bytes --]
On Tue, Mar 21, 2017 at 11:10:22AM +0100, Daniel Vetter wrote:
> On Tue, Mar 21, 2017 at 09:13:54AM +0100, Thierry Reding wrote:
[...]
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
[...]
> > @@ -2437,11 +2476,16 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> > int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> > {
> > struct drm_device *dev = fb_helper->dev;
> > + unsigned int width, height;
> > int err = 0;
> >
> > if (!drm_fbdev_emulation)
> > return 0;
> >
> > + if (fb_helper->deferred_setup)
> > + return drm_fb_helper_initial_config(fb_helper,
> > + fb_helper->preferred_bpp);
>
> I think this must be moved under the protection of ->lock, you might race
> otherwise (e.g. hpd vs. userspace forcing a re-probe, both noticing the
> change).
I think I had originally put this under the lock only to see that result
in a deadlock. I can't quite remember why that was, but testing shows
that this still happens. It's getting rather late, so I'll have to defer
tracking this down to tomorrow.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 06/10] drm/fb-helper: Support deferred setup
2017-03-22 21:06 ` Thierry Reding
@ 2017-03-22 21:08 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-03-22 21:08 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Wed, Mar 22, 2017 at 10:06 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Tue, Mar 21, 2017 at 11:10:22AM +0100, Daniel Vetter wrote:
>> On Tue, Mar 21, 2017 at 09:13:54AM +0100, Thierry Reding wrote:
> [...]
>> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> [...]
>> > @@ -2437,11 +2476,16 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
>> > int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>> > {
>> > struct drm_device *dev = fb_helper->dev;
>> > + unsigned int width, height;
>> > int err = 0;
>> >
>> > if (!drm_fbdev_emulation)
>> > return 0;
>> >
>> > + if (fb_helper->deferred_setup)
>> > + return drm_fb_helper_initial_config(fb_helper,
>> > + fb_helper->preferred_bpp);
>>
>> I think this must be moved under the protection of ->lock, you might race
>> otherwise (e.g. hpd vs. userspace forcing a re-probe, both noticing the
>> change).
>
> I think I had originally put this under the lock only to see that result
> in a deadlock. I can't quite remember why that was, but testing shows
> that this still happens. It's getting rather late, so I'll have to defer
> tracking this down to tomorrow.
initial_config also grabs your new lock, which means you'd need more
static _unlocked variants to make it work. I didn't see any other
obvious problem this would cause.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 06/10] drm/fb-helper: Support deferred setup
2017-03-21 8:13 ` [PATCH v3 06/10] drm/fb-helper: Support deferred setup Thierry Reding
2017-03-21 10:10 ` Daniel Vetter
@ 2017-04-03 8:48 ` Daniel Vetter
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-04-03 8:48 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Tue, Mar 21, 2017 at 09:13:54AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> FB helper code falls back to a 1024x768 mode if no outputs are connected
> or don't report back any modes upon initialization. This can be annoying
> because outputs that are added to FB helper later on can't be used with
> FB helper if they don't support a matching mode.
>
> The fallback is in place because VGA connectors can happen to report an
> unknown connection status even when they are in fact connected.
>
> Some drivers have custom solutions in place to defer FB helper setup
> until at least one output is connected. But the logic behind these
> solutions is always the same and there is nothing driver-specific about
> it, so a better alterative is to fix the FB helper core and add support
> for all drivers automatically.
>
> This patch adds support for deferred FB helper setup. It checks all the
> connectors for their connection status, and if all of them report to be
> disconnected marks the FB helper as needing deferred setup. Whet setup
> is deferred, the FB helper core will automatically retry setup after a
> hotplug event, and it will keep trying until it succeeds.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Ok 2nd attempt at making this work, probably easier to go back to v2.
> ---
> drivers/gpu/drm/drm_fb_helper.c | 60 +++++++++++++++++++++++++++++++++++++----
> include/drm/drm_fb_helper.h | 21 +++++++++++++++
> 2 files changed, 76 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9060adcf7cf8..d4a2c97d8b02 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -511,6 +511,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return -ENODEV;
>
> + if (fb_helper->deferred_setup)
> + return 0;
Please wrap in READ_ONCE to make it clear we're doing lockless checking
here.
> +
> mutex_lock(&fb_helper->lock);
> drm_modeset_lock_all(dev);
>
> @@ -1597,6 +1600,23 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
>
> +static bool drm_fb_helper_maybe_connected(struct drm_fb_helper *helper)
> +{
> + bool connected = false;
> + unsigned int i;
> +
> + for (i = 0; i < helper->connector_count; i++) {
> + struct drm_fb_helper_connector *fb = helper->connector_info[i];
> +
> + if (fb->connector->status != connector_status_disconnected) {
> + connected = true;
> + break;
> + }
> + }
> +
> + return connected;
> +}
> +
> /*
> * Allocates the backing storage and sets up the fbdev info structure through
> * the ->fb_probe callback and then registers the fbdev and sets up the panic
> @@ -2254,8 +2274,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> int i;
>
> DRM_DEBUG_KMS("\n");
> - if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> - DRM_DEBUG_KMS("No connectors reported connected with modes\n");
>
> /* prevent concurrent modification of connector_count by hotplug */
> lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
> @@ -2378,6 +2396,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
> {
> struct drm_device *dev = fb_helper->dev;
> + unsigned int width, height;
> struct fb_info *info;
> int ret;
>
> @@ -2385,14 +2404,34 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
> return 0;
>
From here ...
> mutex_lock(&dev->mode_config.mutex);
> - drm_setup_crtcs(fb_helper,
> - dev->mode_config.max_width,
> - dev->mode_config.max_height);
> +
> + width = dev->mode_config.max_width;
> + height = dev->mode_config.max_height;
> +
> + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> + DRM_DEBUG_KMS("No connectors reported connected with modes\n");
> +
> + /*
> + * If everything's disconnected, there's no use in attempting to set
> + * up fbdev.
> + */
> + if (!drm_fb_helper_maybe_connected(fb_helper)) {
> + DRM_INFO("No outputs connected, deferring setup\n");
> + fb_helper->preferred_bpp = bpp_sel;
> + fb_helper->deferred_setup = true;
> + mutex_unlock(&dev->mode_config.mutex);
> + return 0;
> + }
> +
> + drm_setup_crtcs(fb_helper, width, height);
> +
> ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> mutex_unlock(&dev->mode_config.mutex);
> if (ret)
> return ret;
>
> + fb_helper->deferred_setup = false;
To here you need to hold the new fb_helper->mutex. Plus at the top, after
you've grabbed the lock, you need to re-check ->deferred_setup. This way
we guaranteed that only 1 thread can ever do the deferred setup, while not
holding the lock while we call register_framebuffer. If you hold any fbdev
or kms lock while calling register_framebuffer you will deadlock.
> +
> info = fb_helper->fbdev;
> info->var.pixclock = 0;
> ret = register_framebuffer(info);
> @@ -2437,11 +2476,16 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> + unsigned int width, height;
> int err = 0;
>
> if (!drm_fbdev_emulation)
> return 0;
>
> + if (fb_helper->deferred_setup)
Same here, please wrap in READ_ONCE, this is just a fastpath check.
With those changes your prep patch to roll out fb_helper->mutex more
shouldn't be needed at all.
Cheers, Daniel
> + return drm_fb_helper_initial_config(fb_helper,
> + fb_helper->preferred_bpp);
> +
> mutex_lock(&fb_helper->lock);
> mutex_lock(&dev->mode_config.mutex);
>
> @@ -2453,6 +2497,12 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>
> DRM_DEBUG_KMS("\n");
>
> + width = dev->mode_config.max_width;
> + height = dev->mode_config.max_height;
> +
> + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> + DRM_DEBUG_KMS("No connectors reported connected with modes\n");
> +
> drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
>
> mutex_unlock(&dev->mode_config.mutex);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 33771d0c44e3..41c3587751f8 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -222,6 +222,27 @@ struct drm_fb_helper {
> * needs to be reprobe when fbdev is in control again.
> */
> bool delayed_hotplug;
> +
> + /**
> + * @deferred_setup:
> + *
> + * If no outputs are connected (disconnected or unknown) the FB helper
> + * code will defer setup until at least one of the outputs shows up.
> + * This field keeps track of the status so that setup can be retried
> + * at every hotplug event until it succeeds eventually.
> + */
> + bool deferred_setup;
> +
> + /**
> + * @preferred_bpp:
> + *
> + * Temporary storage for the driver's preferred BPP setting passed to
> + * FB helper initialization. This needs to be tracked so that deferred
> + * FB helper setup can pass this on.
> + *
> + * See also: @deferred_setup
> + */
> + int preferred_bpp;
> };
>
> /**
> --
> 2.12.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (5 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 06/10] drm/fb-helper: Support deferred setup Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 9:58 ` Andrzej Hajda
2017-03-21 8:13 ` [PATCH v3 08/10] drm/hisilicon: " Thierry Reding
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: John Stultz, intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
The FB helper core now supports deferred setup, so the driver's custom
implementation can be removed.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 09d3c4c3c858..c5a37dda8d1b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
/* init kms poll for handling hpd */
drm_kms_helper_poll_init(drm);
- /* force connectors detection */
- drm_helper_hpd_irq_event(drm);
+ ret = exynos_drm_fbdev_init(dev);
+ if (ret)
+ goto err_cleanup_poll;
/* register the DRM device */
ret = drm_dev_register(drm, 0);
@@ -411,6 +412,7 @@ static int exynos_drm_bind(struct device *dev)
err_cleanup_fbdev:
exynos_drm_fbdev_fini(drm);
+err_cleanup_poll:
drm_kms_helper_poll_fini(drm);
exynos_drm_device_subdrv_remove(drm);
err_cleanup_vblank:
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 641531243e04..a020fa70c825 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -306,6 +306,4 @@ void exynos_drm_output_poll_changed(struct drm_device *dev)
if (fb_helper)
drm_fb_helper_hotplug_event(fb_helper);
- else
- exynos_drm_fbdev_init(dev);
}
--
2.12.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 8:13 ` [PATCH v3 07/10] drm/exynos: Remove custom FB helper " Thierry Reding
@ 2017-03-21 9:58 ` Andrzej Hajda
2017-03-21 10:20 ` Daniel Vetter
0 siblings, 1 reply; 31+ messages in thread
From: Andrzej Hajda @ 2017-03-21 9:58 UTC (permalink / raw)
To: Thierry Reding, Daniel Vetter; +Cc: intel-gfx, dri-devel
On 21.03.2017 09:13, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The FB helper core now supports deferred setup, so the driver's custom
> implementation can be removed.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 09d3c4c3c858..c5a37dda8d1b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
> /* init kms poll for handling hpd */
> drm_kms_helper_poll_init(drm);
>
> - /* force connectors detection */
> - drm_helper_hpd_irq_event(drm);
> + ret = exynos_drm_fbdev_init(dev);
> + if (ret)
> + goto err_cleanup_poll;
It should be rather:
ret = exynos_drm_fbdev_init(drm);
Even with this change it does not work, I will try to track down the
problem.
Regards
Andrzej
>
> /* register the DRM device */
> ret = drm_dev_register(drm, 0);
> @@ -411,6 +412,7 @@ static int exynos_drm_bind(struct device *dev)
>
> err_cleanup_fbdev:
> exynos_drm_fbdev_fini(drm);
> +err_cleanup_poll:
> drm_kms_helper_poll_fini(drm);
> exynos_drm_device_subdrv_remove(drm);
> err_cleanup_vblank:
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 641531243e04..a020fa70c825 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -306,6 +306,4 @@ void exynos_drm_output_poll_changed(struct drm_device *dev)
>
> if (fb_helper)
> drm_fb_helper_hotplug_event(fb_helper);
> - else
> - exynos_drm_fbdev_init(dev);
> }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 9:58 ` Andrzej Hajda
@ 2017-03-21 10:20 ` Daniel Vetter
2017-03-21 10:42 ` Andrzej Hajda
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 10:20 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: Daniel Vetter, intel-gfx, Thierry Reding, dri-devel
On Tue, Mar 21, 2017 at 10:58:43AM +0100, Andrzej Hajda wrote:
> On 21.03.2017 09:13, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The FB helper core now supports deferred setup, so the driver's custom
> > implementation can be removed.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
> > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > index 09d3c4c3c858..c5a37dda8d1b 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
> > /* init kms poll for handling hpd */
> > drm_kms_helper_poll_init(drm);
> >
> > - /* force connectors detection */
> > - drm_helper_hpd_irq_event(drm);
> > + ret = exynos_drm_fbdev_init(dev);
> > + if (ret)
> > + goto err_cleanup_poll;
>
> It should be rather:
> ret = exynos_drm_fbdev_init(drm);
>
> Even with this change it does not work, I will try to track down the
> problem.
We might need the multi-stage fbdev setup here, i.e. init fbdev, run
hpd_irq_event() (I suspect that kicks all the connectors to start
probing), then initial_config for fbdev.
Probably simplest way to achieve this is to keep the hpd_irq_event call,
but place it _after_ the fbdev_init.
-Daniel
>
> Regards
> Andrzej
>
>
> >
> > /* register the DRM device */
> > ret = drm_dev_register(drm, 0);
> > @@ -411,6 +412,7 @@ static int exynos_drm_bind(struct device *dev)
> >
> > err_cleanup_fbdev:
> > exynos_drm_fbdev_fini(drm);
> > +err_cleanup_poll:
> > drm_kms_helper_poll_fini(drm);
> > exynos_drm_device_subdrv_remove(drm);
> > err_cleanup_vblank:
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> > index 641531243e04..a020fa70c825 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> > @@ -306,6 +306,4 @@ void exynos_drm_output_poll_changed(struct drm_device *dev)
> >
> > if (fb_helper)
> > drm_fb_helper_hotplug_event(fb_helper);
> > - else
> > - exynos_drm_fbdev_init(dev);
> > }
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 10:20 ` Daniel Vetter
@ 2017-03-21 10:42 ` Andrzej Hajda
2017-03-21 10:53 ` Thierry Reding
0 siblings, 1 reply; 31+ messages in thread
From: Andrzej Hajda @ 2017-03-21 10:42 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, intel-gfx, Thierry Reding, dri-devel
On 21.03.2017 11:20, Daniel Vetter wrote:
> On Tue, Mar 21, 2017 at 10:58:43AM +0100, Andrzej Hajda wrote:
>> On 21.03.2017 09:13, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> The FB helper core now supports deferred setup, so the driver's custom
>>> implementation can be removed.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
>>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
>>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> index 09d3c4c3c858..c5a37dda8d1b 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
>>> /* init kms poll for handling hpd */
>>> drm_kms_helper_poll_init(drm);
>>>
>>> - /* force connectors detection */
>>> - drm_helper_hpd_irq_event(drm);
>>> + ret = exynos_drm_fbdev_init(dev);
>>> + if (ret)
>>> + goto err_cleanup_poll;
>> It should be rather:
>> ret = exynos_drm_fbdev_init(drm);
>>
>> Even with this change it does not work, I will try to track down the
>> problem.
The solution was to remove custom equivalent of
drm_fb_helper_maybe_connected - exynos_drm_fbdev_is_anything_connected,
it is called earlier and it was a part of custom deferred fbdev.
> We might need the multi-stage fbdev setup here, i.e. init fbdev, run
> hpd_irq_event() (I suspect that kicks all the connectors to start
> probing), then initial_config for fbdev.
>
> Probably simplest way to achieve this is to keep the hpd_irq_event call,
> but place it _after_ the fbdev_init.
> -Daniel
I wonder if it wouldn't be sufficient to check if there is anything
connected, instead of checking if there is anything not-disconnected, in
drm_fb_helper_maybe_connected.
Regards
Andrzej
>
>> Regards
>> Andrzej
>>
>>
>>>
>>> /* register the DRM device */
>>> ret = drm_dev_register(drm, 0);
>>> @@ -411,6 +412,7 @@ static int exynos_drm_bind(struct device *dev)
>>>
>>> err_cleanup_fbdev:
>>> exynos_drm_fbdev_fini(drm);
>>> +err_cleanup_poll:
>>> drm_kms_helper_poll_fini(drm);
>>> exynos_drm_device_subdrv_remove(drm);
>>> err_cleanup_vblank:
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> index 641531243e04..a020fa70c825 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> @@ -306,6 +306,4 @@ void exynos_drm_output_poll_changed(struct drm_device *dev)
>>>
>>> if (fb_helper)
>>> drm_fb_helper_hotplug_event(fb_helper);
>>> - else
>>> - exynos_drm_fbdev_init(dev);
>>> }
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 10:42 ` Andrzej Hajda
@ 2017-03-21 10:53 ` Thierry Reding
2017-03-21 11:13 ` Andrzej Hajda
0 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 10:53 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: Daniel Vetter, intel-gfx, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2587 bytes --]
On Tue, Mar 21, 2017 at 11:42:21AM +0100, Andrzej Hajda wrote:
> On 21.03.2017 11:20, Daniel Vetter wrote:
> > On Tue, Mar 21, 2017 at 10:58:43AM +0100, Andrzej Hajda wrote:
> >> On 21.03.2017 09:13, Thierry Reding wrote:
> >>> From: Thierry Reding <treding@nvidia.com>
> >>>
> >>> The FB helper core now supports deferred setup, so the driver's custom
> >>> implementation can be removed.
> >>>
> >>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>> ---
> >>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
> >>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
> >>> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>> index 09d3c4c3c858..c5a37dda8d1b 100644
> >>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>> @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
> >>> /* init kms poll for handling hpd */
> >>> drm_kms_helper_poll_init(drm);
> >>>
> >>> - /* force connectors detection */
> >>> - drm_helper_hpd_irq_event(drm);
> >>> + ret = exynos_drm_fbdev_init(dev);
> >>> + if (ret)
> >>> + goto err_cleanup_poll;
> >> It should be rather:
> >> ret = exynos_drm_fbdev_init(drm);
> >>
> >> Even with this change it does not work, I will try to track down the
> >> problem.
>
> The solution was to remove custom equivalent of
> drm_fb_helper_maybe_connected - exynos_drm_fbdev_is_anything_connected,
> it is called earlier and it was a part of custom deferred fbdev.
Cool, thanks for figuring that out. I can add that to this patch for v4.
> > We might need the multi-stage fbdev setup here, i.e. init fbdev, run
> > hpd_irq_event() (I suspect that kicks all the connectors to start
> > probing), then initial_config for fbdev.
> >
> > Probably simplest way to achieve this is to keep the hpd_irq_event call,
> > but place it _after_ the fbdev_init.
> > -Daniel
>
> I wonder if it wouldn't be sufficient to check if there is anything
> connected, instead of checking if there is anything not-disconnected, in
> drm_fb_helper_maybe_connected.
My recollection is that this had been discussed when I first sent this
out. VGA outputs are somewhat of a special case in that you can't always
properly detect that it's connected or not. So effectively we need to
take into account the connector_status_unknown. There's also some more
information about this in the kerneldoc for enum drm_connector_status.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 10:53 ` Thierry Reding
@ 2017-03-21 11:13 ` Andrzej Hajda
2017-03-21 11:17 ` Thierry Reding
2017-03-21 12:28 ` Daniel Vetter
0 siblings, 2 replies; 31+ messages in thread
From: Andrzej Hajda @ 2017-03-21 11:13 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On 21.03.2017 11:53, Thierry Reding wrote:
> On Tue, Mar 21, 2017 at 11:42:21AM +0100, Andrzej Hajda wrote:
>> On 21.03.2017 11:20, Daniel Vetter wrote:
>>> On Tue, Mar 21, 2017 at 10:58:43AM +0100, Andrzej Hajda wrote:
>>>> On 21.03.2017 09:13, Thierry Reding wrote:
>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>
>>>>> The FB helper core now supports deferred setup, so the driver's custom
>>>>> implementation can be removed.
>>>>>
>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>>> ---
>>>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
>>>>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
>>>>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>>>> index 09d3c4c3c858..c5a37dda8d1b 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>>>> @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
>>>>> /* init kms poll for handling hpd */
>>>>> drm_kms_helper_poll_init(drm);
>>>>>
>>>>> - /* force connectors detection */
>>>>> - drm_helper_hpd_irq_event(drm);
>>>>> + ret = exynos_drm_fbdev_init(dev);
>>>>> + if (ret)
>>>>> + goto err_cleanup_poll;
>>>> It should be rather:
>>>> ret = exynos_drm_fbdev_init(drm);
>>>>
>>>> Even with this change it does not work, I will try to track down the
>>>> problem.
>> The solution was to remove custom equivalent of
>> drm_fb_helper_maybe_connected - exynos_drm_fbdev_is_anything_connected,
>> it is called earlier and it was a part of custom deferred fbdev.
> Cool, thanks for figuring that out. I can add that to this patch for v4.
>
>>> We might need the multi-stage fbdev setup here, i.e. init fbdev, run
>>> hpd_irq_event() (I suspect that kicks all the connectors to start
>>> probing), then initial_config for fbdev.
>>>
>>> Probably simplest way to achieve this is to keep the hpd_irq_event call,
>>> but place it _after_ the fbdev_init.
>>> -Daniel
>> I wonder if it wouldn't be sufficient to check if there is anything
>> connected, instead of checking if there is anything not-disconnected, in
>> drm_fb_helper_maybe_connected.
> My recollection is that this had been discussed when I first sent this
> out. VGA outputs are somewhat of a special case in that you can't always
> properly detect that it's connected or not. So effectively we need to
> take into account the connector_status_unknown. There's also some more
> information about this in the kerneldoc for enum drm_connector_status.
OK, I forgot about it and assumed that it is only used for initial state
of connector.
Returning to Daniel's proposition about hpd_irq_event(), it seems
unnecessary then: before calling drm_fb_helper_maybe_connected
drm_fb_helper_probe_connector_modes is called, which calls fill_modes
callback which should perform connector probing anyway, am I right?
Regards
Andrzej
>
> Thierry
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 11:13 ` Andrzej Hajda
@ 2017-03-21 11:17 ` Thierry Reding
2017-03-21 12:28 ` Daniel Vetter
1 sibling, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 11:17 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: Daniel Vetter, intel-gfx, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 3249 bytes --]
On Tue, Mar 21, 2017 at 12:13:26PM +0100, Andrzej Hajda wrote:
> On 21.03.2017 11:53, Thierry Reding wrote:
> > On Tue, Mar 21, 2017 at 11:42:21AM +0100, Andrzej Hajda wrote:
> >> On 21.03.2017 11:20, Daniel Vetter wrote:
> >>> On Tue, Mar 21, 2017 at 10:58:43AM +0100, Andrzej Hajda wrote:
> >>>> On 21.03.2017 09:13, Thierry Reding wrote:
> >>>>> From: Thierry Reding <treding@nvidia.com>
> >>>>>
> >>>>> The FB helper core now supports deferred setup, so the driver's custom
> >>>>> implementation can be removed.
> >>>>>
> >>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>>>> ---
> >>>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
> >>>>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
> >>>>> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>>>> index 09d3c4c3c858..c5a37dda8d1b 100644
> >>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>>>> @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
> >>>>> /* init kms poll for handling hpd */
> >>>>> drm_kms_helper_poll_init(drm);
> >>>>>
> >>>>> - /* force connectors detection */
> >>>>> - drm_helper_hpd_irq_event(drm);
> >>>>> + ret = exynos_drm_fbdev_init(dev);
> >>>>> + if (ret)
> >>>>> + goto err_cleanup_poll;
> >>>> It should be rather:
> >>>> ret = exynos_drm_fbdev_init(drm);
> >>>>
> >>>> Even with this change it does not work, I will try to track down the
> >>>> problem.
> >> The solution was to remove custom equivalent of
> >> drm_fb_helper_maybe_connected - exynos_drm_fbdev_is_anything_connected,
> >> it is called earlier and it was a part of custom deferred fbdev.
> > Cool, thanks for figuring that out. I can add that to this patch for v4.
> >
> >>> We might need the multi-stage fbdev setup here, i.e. init fbdev, run
> >>> hpd_irq_event() (I suspect that kicks all the connectors to start
> >>> probing), then initial_config for fbdev.
> >>>
> >>> Probably simplest way to achieve this is to keep the hpd_irq_event call,
> >>> but place it _after_ the fbdev_init.
> >>> -Daniel
> >> I wonder if it wouldn't be sufficient to check if there is anything
> >> connected, instead of checking if there is anything not-disconnected, in
> >> drm_fb_helper_maybe_connected.
> > My recollection is that this had been discussed when I first sent this
> > out. VGA outputs are somewhat of a special case in that you can't always
> > properly detect that it's connected or not. So effectively we need to
> > take into account the connector_status_unknown. There's also some more
> > information about this in the kerneldoc for enum drm_connector_status.
>
> OK, I forgot about it and assumed that it is only used for initial state
> of connector.
> Returning to Daniel's proposition about hpd_irq_event(), it seems
> unnecessary then: before calling drm_fb_helper_maybe_connected
> drm_fb_helper_probe_connector_modes is called, which calls fill_modes
> callback which should perform connector probing anyway, am I right?
Yes, that matches my understanding of the call sequence.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup
2017-03-21 11:13 ` Andrzej Hajda
2017-03-21 11:17 ` Thierry Reding
@ 2017-03-21 12:28 ` Daniel Vetter
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 12:28 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: Daniel Vetter, intel-gfx, Thierry Reding, dri-devel
On Tue, Mar 21, 2017 at 12:13:26PM +0100, Andrzej Hajda wrote:
> On 21.03.2017 11:53, Thierry Reding wrote:
> > On Tue, Mar 21, 2017 at 11:42:21AM +0100, Andrzej Hajda wrote:
> >> On 21.03.2017 11:20, Daniel Vetter wrote:
> >>> On Tue, Mar 21, 2017 at 10:58:43AM +0100, Andrzej Hajda wrote:
> >>>> On 21.03.2017 09:13, Thierry Reding wrote:
> >>>>> From: Thierry Reding <treding@nvidia.com>
> >>>>>
> >>>>> The FB helper core now supports deferred setup, so the driver's custom
> >>>>> implementation can be removed.
> >>>>>
> >>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>>>> ---
> >>>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++--
> >>>>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
> >>>>> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>>>> index 09d3c4c3c858..c5a37dda8d1b 100644
> >>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> >>>>> @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
> >>>>> /* init kms poll for handling hpd */
> >>>>> drm_kms_helper_poll_init(drm);
> >>>>>
> >>>>> - /* force connectors detection */
> >>>>> - drm_helper_hpd_irq_event(drm);
> >>>>> + ret = exynos_drm_fbdev_init(dev);
> >>>>> + if (ret)
> >>>>> + goto err_cleanup_poll;
> >>>> It should be rather:
> >>>> ret = exynos_drm_fbdev_init(drm);
> >>>>
> >>>> Even with this change it does not work, I will try to track down the
> >>>> problem.
> >> The solution was to remove custom equivalent of
> >> drm_fb_helper_maybe_connected - exynos_drm_fbdev_is_anything_connected,
> >> it is called earlier and it was a part of custom deferred fbdev.
> > Cool, thanks for figuring that out. I can add that to this patch for v4.
> >
> >>> We might need the multi-stage fbdev setup here, i.e. init fbdev, run
> >>> hpd_irq_event() (I suspect that kicks all the connectors to start
> >>> probing), then initial_config for fbdev.
> >>>
> >>> Probably simplest way to achieve this is to keep the hpd_irq_event call,
> >>> but place it _after_ the fbdev_init.
> >>> -Daniel
> >> I wonder if it wouldn't be sufficient to check if there is anything
> >> connected, instead of checking if there is anything not-disconnected, in
> >> drm_fb_helper_maybe_connected.
> > My recollection is that this had been discussed when I first sent this
> > out. VGA outputs are somewhat of a special case in that you can't always
> > properly detect that it's connected or not. So effectively we need to
> > take into account the connector_status_unknown. There's also some more
> > information about this in the kerneldoc for enum drm_connector_status.
>
> OK, I forgot about it and assumed that it is only used for initial state
> of connector.
unknown does indeed mean "I couldn't figure this out", which kinda only
happesn for VGA on platforms where load detect doesn't exist (or doesn't
reliably exist). We should probably document this somewhere ... hint :-)
> Returning to Daniel's proposition about hpd_irq_event(), it seems
> unnecessary then: before calling drm_fb_helper_maybe_connected
> drm_fb_helper_probe_connector_modes is called, which calls fill_modes
> callback which should perform connector probing anyway, am I right?
Yeah if you can fix this by removing the exynos copy of maybe_connected,
then we indeed don't need the hpd_irq. Needing it would indicate a bug in
exynos anyway.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 08/10] drm/hisilicon: Remove custom FB helper deferred setup
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (6 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 07/10] drm/exynos: Remove custom FB helper " Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 8:13 ` [PATCH v3 09/10] drm/atmel-hlcdc: Remove unnecessary NULL check Thierry Reding
` (2 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
The FB helper core now supports deferred setup, so the driver's custom
implementation can be removed.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index df4f50713e54..408c7cfc180c 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -55,14 +55,7 @@ static void kirin_fbdev_output_poll_changed(struct drm_device *dev)
{
struct kirin_drm_private *priv = dev->dev_private;
- if (priv->fbdev) {
- drm_fbdev_cma_hotplug_event(priv->fbdev);
- } else {
- priv->fbdev = drm_fbdev_cma_init(dev, 32,
- dev->mode_config.num_connector);
- if (IS_ERR(priv->fbdev))
- priv->fbdev = NULL;
- }
+ drm_fbdev_cma_hotplug_event(priv->fbdev);
}
#endif
@@ -129,11 +122,19 @@ static int kirin_drm_kms_init(struct drm_device *dev)
/* init kms poll for handling hpd */
drm_kms_helper_poll_init(dev);
- /* force detection after connectors init */
- (void)drm_helper_hpd_irq_event(dev);
+ priv->fbdev = drm_fbdev_cma_init(dev, 32,
+ dev->mode_config.num_connector);
+ if (IS_ERR(priv->fbdev)) {
+ DRM_ERROR("failed to initialize fbdev.\n");
+ ret = PTR_ERR(priv->fbdev);
+ goto err_cleanup_poll;
+ }
return 0;
+err_cleanup_poll:
+ drm_kms_helper_poll_fini(dev);
+ drm_vblank_cleanup(dev);
err_unbind_all:
component_unbind_all(dev->dev, dev);
err_dc_cleanup:
--
2.12.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 09/10] drm/atmel-hlcdc: Remove unnecessary NULL check
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (7 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 08/10] drm/hisilicon: " Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 8:13 ` [PATCH v3 10/10] drm/rockchip: " Thierry Reding
2017-03-21 9:57 ` ✓ Fi.CI.BAT: success for drm/fb-helper: Deferred setup support (rev2) Patchwork
10 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
drm_fbdev_cma_hotplug_event() already checks for NULL pointers before
dereferencing, so callers don't need to do that.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index f4a3065f7f51..31443402b9a9 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -450,8 +450,7 @@ static void atmel_hlcdc_fb_output_poll_changed(struct drm_device *dev)
{
struct atmel_hlcdc_dc *dc = dev->dev_private;
- if (dc->fbdev)
- drm_fbdev_cma_hotplug_event(dc->fbdev);
+ drm_fbdev_cma_hotplug_event(dc->fbdev);
}
struct atmel_hlcdc_dc_commit {
--
2.12.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 10/10] drm/rockchip: Remove unnecessary NULL check
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (8 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 09/10] drm/atmel-hlcdc: Remove unnecessary NULL check Thierry Reding
@ 2017-03-21 8:13 ` Thierry Reding
2017-03-21 10:27 ` Daniel Vetter
2017-03-21 9:57 ` ✓ Fi.CI.BAT: success for drm/fb-helper: Deferred setup support (rev2) Patchwork
10 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 8:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: John Stultz, intel-gfx, dri-devel
From: Thierry Reding <treding@nvidia.com>
The expression &private->fbdev_helper can never be NULL, so the check is
completely unnecessary.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 81f9548672b0..df6bceabeca8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -168,10 +168,8 @@ rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
static void rockchip_drm_output_poll_changed(struct drm_device *dev)
{
struct rockchip_drm_private *private = dev->dev_private;
- struct drm_fb_helper *fb_helper = &private->fbdev_helper;
- if (fb_helper)
- drm_fb_helper_hotplug_event(fb_helper);
+ drm_fb_helper_hotplug_event(&private->fbdev_helper);
}
static void
--
2.12.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 10/10] drm/rockchip: Remove unnecessary NULL check
2017-03-21 8:13 ` [PATCH v3 10/10] drm/rockchip: " Thierry Reding
@ 2017-03-21 10:27 ` Daniel Vetter
2017-03-21 11:13 ` Thierry Reding
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2017-03-21 10:27 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Tue, Mar 21, 2017 at 09:13:58AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The expression &private->fbdev_helper can never be NULL, so the check is
> completely unnecessary.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
On the 3 drivers patches (all except exynos):
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For merging, do you want to push it all through drm-misc? Or planning on
sending a pull request to Dave?
-Daniel
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 81f9548672b0..df6bceabeca8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -168,10 +168,8 @@ rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
> static void rockchip_drm_output_poll_changed(struct drm_device *dev)
> {
> struct rockchip_drm_private *private = dev->dev_private;
> - struct drm_fb_helper *fb_helper = &private->fbdev_helper;
>
> - if (fb_helper)
> - drm_fb_helper_hotplug_event(fb_helper);
> + drm_fb_helper_hotplug_event(&private->fbdev_helper);
> }
>
> static void
> --
> 2.12.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 10/10] drm/rockchip: Remove unnecessary NULL check
2017-03-21 10:27 ` Daniel Vetter
@ 2017-03-21 11:13 ` Thierry Reding
0 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2017-03-21 11:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, intel-gfx, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 746 bytes --]
On Tue, Mar 21, 2017 at 11:27:04AM +0100, Daniel Vetter wrote:
> On Tue, Mar 21, 2017 at 09:13:58AM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The expression &private->fbdev_helper can never be NULL, so the check is
> > completely unnecessary.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
>
> On the 3 drivers patches (all except exynos):
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> For merging, do you want to push it all through drm-misc? Or planning on
> sending a pull request to Dave?
I think this is perfect material for drm-misc. I'll resend v4 shortly
with a build fix and the fix Andrzej pointed out folded into the Exynos
driver.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* ✓ Fi.CI.BAT: success for drm/fb-helper: Deferred setup support (rev2)
2017-03-21 8:13 [PATCH v3 00/10] drm/fb-helper: Deferred setup support Thierry Reding
` (9 preceding siblings ...)
2017-03-21 8:13 ` [PATCH v3 10/10] drm/rockchip: " Thierry Reding
@ 2017-03-21 9:57 ` Patchwork
10 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2017-03-21 9:57 UTC (permalink / raw)
To: Thierry Reding; +Cc: intel-gfx
== Series Details ==
Series: drm/fb-helper: Deferred setup support (rev2)
URL : https://patchwork.freedesktop.org/series/8410/
State : success
== Summary ==
Series 8410v2 drm/fb-helper: Deferred setup support
https://patchwork.freedesktop.org/api/1.0/series/8410/revisions/2/mbox/
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 458s
fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 601s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 535s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 581s
fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 511s
fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 503s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 441s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 439s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 510s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 499s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 489s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 484s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 602s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 486s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 516s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 554s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 413s
5b6f03e54c55e73e102b34138242d646c7afd829 drm-tip: 2017y-03m-21d-08h-31m-10s UTC integration manifest
6b2f5da drm/rockchip: Remove unnecessary NULL check
fbd6b26 drm/atmel-hlcdc: Remove unnecessary NULL check
965e81a drm/hisilicon: Remove custom FB helper deferred setup
04d935a drm/exynos: Remove custom FB helper deferred setup
c94913a drm/fb-helper: Support deferred setup
be4ed65 drm/fb-helper: Add top-level lock
cf6ec86 drm/fb-helper: Push down modeset lock into FB helpers
c310d30 drm/fb-helper: Improve code readability
cf64817 drm/fb-helper: Reshuffle code for subsequent patches
0fd2dd3 drm/fb-helper: Cleanup checkpatch warnings
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4238/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread