* [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init()
@ 2022-03-22 13:17 Patrik Jakobsson
2022-03-22 13:17 ` [PATCH 2/7] drm/gma500: Uninstall interrupts on driver removal Patrik Jakobsson
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Patrik Jakobsson @ 2022-03-22 13:17 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, sam, tzimmermann
Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
drivers/gpu/drm/gma500/framebuffer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 45df9de22007..2b99c996fdc2 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev)
struct pci_dev *pdev = to_pci_dev(dev->dev);
int i;
- drm_mode_config_init(dev);
+ if (drmm_mode_config_init(dev))
+ return;
dev->mode_config.min_width = 0;
dev->mode_config.min_height = 0;
@@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev)
if (dev_priv->modeset) {
drm_kms_helper_poll_fini(dev);
psb_fbdev_fini(dev);
- drm_mode_config_cleanup(dev);
}
}
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/7] drm/gma500: Uninstall interrupts on driver removal 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson @ 2022-03-22 13:17 ` Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 3/7] drm/gma500: Make use of the drm connector iterator Patrik Jakobsson ` (5 subsequent siblings) 6 siblings, 0 replies; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 13:17 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, sam, tzimmermann Reloading the driver revealed that the interrupt handler never got uninstalled. Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> --- drivers/gpu/drm/gma500/psb_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 82d51e9821ad..b231fddb8817 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -173,6 +173,8 @@ static void psb_driver_unload(struct drm_device *dev) gma_backlight_exit(dev); psb_modeset_cleanup(dev); + gma_irq_uninstall(dev); + if (dev_priv->ops->chip_teardown) dev_priv->ops->chip_teardown(dev); -- 2.35.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/7] drm/gma500: Make use of the drm connector iterator 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 2/7] drm/gma500: Uninstall interrupts on driver removal Patrik Jakobsson @ 2022-03-22 13:17 ` Patrik Jakobsson 2022-03-22 14:39 ` Daniel Vetter 2022-03-22 13:17 ` [PATCH 4/7] drm/gma500: gma500 don't register non-hotpluggable connectors Patrik Jakobsson ` (4 subsequent siblings) 6 siblings, 1 reply; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 13:17 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, sam, tzimmermann This makes sure we're using proper locking when iterating the list of connectors. Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> --- drivers/gpu/drm/gma500/cdv_device.c | 10 ++++++-- drivers/gpu/drm/gma500/cdv_intel_display.c | 9 +++++-- drivers/gpu/drm/gma500/framebuffer.c | 6 +++-- drivers/gpu/drm/gma500/gma_display.c | 16 ++++++++----- drivers/gpu/drm/gma500/oaktrail_crtc.c | 17 ++++++++----- drivers/gpu/drm/gma500/oaktrail_lvds.c | 15 ++++++------ drivers/gpu/drm/gma500/psb_device.c | 28 +++++++++++++++------- drivers/gpu/drm/gma500/psb_drv.c | 10 ++++---- drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++---- 9 files changed, 84 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c index f854f58bcbb3..dd32b484dd82 100644 --- a/drivers/gpu/drm/gma500/cdv_device.c +++ b/drivers/gpu/drm/gma500/cdv_device.c @@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev) struct drm_psb_private *dev_priv = to_drm_psb_private(dev); struct pci_dev *pdev = to_pci_dev(dev->dev); struct psb_save_area *regs = &dev_priv->regs; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; dev_dbg(dev->dev, "Saving GPU registers.\n"); @@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev) regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R); regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R); - list_for_each_entry(connector, &dev->mode_config.connector_list, head) + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF); + drm_connector_list_iter_end(&conn_iter); return 0; } @@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev) struct drm_psb_private *dev_priv = to_drm_psb_private(dev); struct pci_dev *pdev = to_pci_dev(dev->dev); struct psb_save_area *regs = &dev_priv->regs; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; u32 temp; @@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev) drm_mode_config_reset(dev); - list_for_each_entry(connector, &dev->mode_config.connector_list, head) + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); + drm_connector_list_iter_end(&conn_iter); /* Resume the modeset for every activated CRTC */ drm_helper_resume_force_mode(dev); diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c index 94ebc48a4349..0c3ddcdc29dc 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_display.c +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c @@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, bool ok; bool is_lvds = false; bool is_dp = false; - struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; const struct gma_limit_t *limit; u32 ddi_select = 0; bool is_edp = false; - list_for_each_entry(connector, &mode_config->connector_list, head) { + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector); @@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, is_edp = true; break; default: + drm_connector_list_iter_end(&conn_iter); DRM_ERROR("invalid output type.\n"); return 0; } + + break; } + drm_connector_list_iter_end(&conn_iter); if (dev_priv->dplla_96mhz) /* low-end sku, 96/100 mhz */ diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 2b99c996fdc2..0ac6ea5fd3a1 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = { static void psb_setup_outputs(struct drm_device *dev) { struct drm_psb_private *dev_priv = to_drm_psb_private(dev); + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; drm_mode_create_scaling_mode_property(dev); @@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev) "backlight", 0, 100); dev_priv->ops->output_init(dev); - list_for_each_entry(connector, &dev->mode_config.connector_list, - head) { + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector); struct drm_encoder *encoder = &gma_encoder->base; int crtc_mask = 0, clone_mask = 0; @@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev) encoder->possible_clones = gma_connector_clones(dev, clone_mask); } + drm_connector_list_iter_end(&conn_iter); } void psb_modeset_init(struct drm_device *dev) diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c index 1d7964c339f4..e8157464d9eb 100644 --- a/drivers/gpu/drm/gma500/gma_display.c +++ b/drivers/gpu/drm/gma500/gma_display.c @@ -27,17 +27,21 @@ bool gma_pipe_has_type(struct drm_crtc *crtc, int type) { struct drm_device *dev = crtc->dev; - struct drm_mode_config *mode_config = &dev->mode_config; - struct drm_connector *l_entry; + struct drm_connector_list_iter conn_iter; + struct drm_connector *connector; - list_for_each_entry(l_entry, &mode_config->connector_list, head) { - if (l_entry->encoder && l_entry->encoder->crtc == crtc) { + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { + if (connector->encoder && connector->encoder->crtc == crtc) { struct gma_encoder *gma_encoder = - gma_attached_encoder(l_entry); - if (gma_encoder->type == type) + gma_attached_encoder(connector); + if (gma_encoder->type == type) { + drm_connector_list_iter_end(&conn_iter); return true; + } } } + drm_connector_list_iter_end(&conn_iter); return false; } diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c index 36c7c2686c90..873c17cf8fb4 100644 --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c @@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, bool ok, is_sdvo = false; bool is_lvds = false; bool is_mipi = false; - struct drm_mode_config *mode_config = &dev->mode_config; struct gma_encoder *gma_encoder = NULL; uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; int i; int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0; @@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, adjusted_mode, sizeof(struct drm_display_mode)); - list_for_each_entry(connector, &mode_config->connector_list, head) { + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { if (!connector->encoder || connector->encoder->crtc != crtc) continue; @@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, is_mipi = true; break; } + + break; } + if (gma_encoder) + drm_object_property_get_value(&connector->base, + dev->mode_config.scaling_mode_property, &scalingType); + + drm_connector_list_iter_end(&conn_iter); + /* Disable the VGA plane that we never use */ for (i = 0; i <= need_aux; i++) REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i); @@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, (mode->crtc_vdisplay - 1), i); } - if (gma_encoder) - drm_object_property_get_value(&connector->base, - dev->mode_config.scaling_mode_property, &scalingType); - if (scalingType == DRM_MODE_SCALE_NO_SCALE) { /* Moorestown doesn't have register support for centering so * we need to mess with the h/vblank and h/vsync start and diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c index 28b995ef2844..04852dbc7fb3 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c @@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, struct drm_device *dev = encoder->dev; struct drm_psb_private *dev_priv = to_drm_psb_private(dev); struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev; - struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector = NULL; struct drm_crtc *crtc = encoder->crtc; u32 lvds_port; @@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, REG_WRITE(LVDS, lvds_port); /* Find the connector we're trying to set up */ - list_for_each_entry(connector, &mode_config->connector_list, head) { + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { if (connector->encoder && connector->encoder->crtc == crtc) break; } - if (list_entry_is_head(connector, &mode_config->connector_list, head)) { + if (!connector) { + drm_connector_list_iter_end(&conn_iter); DRM_ERROR("Couldn't find connector when setting mode"); gma_power_end(dev); return; } - drm_object_property_get_value( - &connector->base, - dev->mode_config.scaling_mode_property, - &v); + drm_object_property_get_value( &connector->base, + dev->mode_config.scaling_mode_property, &v); + drm_connector_list_iter_end(&conn_iter); if (v == DRM_MODE_SCALE_NO_SCALE) REG_WRITE(PFIT_CONTROL, 0); diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c index 59f325165667..71534f4ca834 100644 --- a/drivers/gpu/drm/gma500/psb_device.c +++ b/drivers/gpu/drm/gma500/psb_device.c @@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev) static int psb_save_display_registers(struct drm_device *dev) { struct drm_psb_private *dev_priv = to_drm_psb_private(dev); + struct gma_connector *gma_connector; struct drm_crtc *crtc; - struct gma_connector *connector; + struct drm_connector_list_iter conn_iter; + struct drm_connector *connector; struct psb_state *regs = &dev_priv->regs.psb; /* Display arbitration control + watermarks */ @@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev) dev_priv->ops->save_crtc(crtc); } - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) - if (connector->save) - connector->save(&connector->base); + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { + gma_connector = to_gma_connector(connector); + if (gma_connector->save) + gma_connector->save(connector); + } + drm_connector_list_iter_end(&conn_iter); drm_modeset_unlock_all(dev); return 0; @@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev) static int psb_restore_display_registers(struct drm_device *dev) { struct drm_psb_private *dev_priv = to_drm_psb_private(dev); + struct gma_connector *gma_connector; struct drm_crtc *crtc; - struct gma_connector *connector; + struct drm_connector_list_iter conn_iter; + struct drm_connector *connector; struct psb_state *regs = &dev_priv->regs.psb; /* Display arbitration + watermarks */ @@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev) if (drm_helper_crtc_in_use(crtc)) dev_priv->ops->restore_crtc(crtc); - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) - if (connector->restore) - connector->restore(&connector->base); + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { + gma_connector = to_gma_connector(connector); + if (gma_connector->restore) + gma_connector->restore(connector); + } + drm_connector_list_iter_end(&conn_iter); drm_modeset_unlock_all(dev); return 0; diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index b231fddb8817..bb0e3288e35b 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) struct drm_psb_private *dev_priv = to_drm_psb_private(dev); unsigned long resource_start, resource_len; unsigned long irqflags; - int ret = -ENOMEM; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; struct gma_encoder *gma_encoder; struct psb_gtt *pg; + int ret = -ENOMEM; /* initializing driver private data */ @@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) psb_fbdev_init(dev); drm_kms_helper_poll_init(dev); - /* Only add backlight support if we have LVDS output */ - list_for_each_entry(connector, &dev->mode_config.connector_list, - head) { + /* Only add backlight support if we have LVDS or MIPI output */ + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { gma_encoder = gma_attached_encoder(connector); switch (gma_encoder->type) { @@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) break; } } + drm_connector_list_iter_end(&conn_iter); if (ret) return ret; diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c index a99859b5b13a..fb8234f4d128 100644 --- a/drivers/gpu/drm/gma500/psb_intel_display.c +++ b/drivers/gpu/drm/gma500/psb_intel_display.c @@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, u32 dpll = 0, fp = 0, dspcntr, pipeconf; bool ok, is_sdvo = false; bool is_lvds = false, is_tv = false; - struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; const struct gma_limit_t *limit; @@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, return 0; } - list_for_each_entry(connector, &mode_config->connector_list, head) { + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector); if (!connector->encoder @@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, break; } } + drm_connector_list_iter_end(&conn_iter); refclk = 96000; @@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe) int gma_connector_clones(struct drm_device *dev, int type_mask) { - int index_mask = 0; + struct drm_connector_list_iter conn_iter; struct drm_connector *connector; + int index_mask = 0; int entry = 0; - list_for_each_entry(connector, &dev->mode_config.connector_list, - head) { + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector); if (type_mask & (1 << gma_encoder->type)) index_mask |= (1 << entry); entry++; } + drm_connector_list_iter_end(&conn_iter); + return index_mask; } -- 2.35.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator 2022-03-22 13:17 ` [PATCH 3/7] drm/gma500: Make use of the drm connector iterator Patrik Jakobsson @ 2022-03-22 14:39 ` Daniel Vetter 2022-03-22 15:46 ` Patrik Jakobsson 0 siblings, 1 reply; 15+ messages in thread From: Daniel Vetter @ 2022-03-22 14:39 UTC (permalink / raw) To: Patrik Jakobsson; +Cc: daniel.vetter, sam, tzimmermann, dri-devel On Tue, Mar 22, 2022 at 02:17:38PM +0100, Patrik Jakobsson wrote: > This makes sure we're using proper locking when iterating the list of > connectors. > > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> Note that this is only needed if your driver deals with hotpluggable connectors. Since gma500 doesn't, not really a need to convert this over, but it also doesn't hurt. If the kerneldoc doesn't explain this, maybe we should add it? Care for a patch. Either way on the entire series: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> I'll leave it up to you whether you want to push this one too or not. -Daniel > --- > drivers/gpu/drm/gma500/cdv_device.c | 10 ++++++-- > drivers/gpu/drm/gma500/cdv_intel_display.c | 9 +++++-- > drivers/gpu/drm/gma500/framebuffer.c | 6 +++-- > drivers/gpu/drm/gma500/gma_display.c | 16 ++++++++----- > drivers/gpu/drm/gma500/oaktrail_crtc.c | 17 ++++++++----- > drivers/gpu/drm/gma500/oaktrail_lvds.c | 15 ++++++------ > drivers/gpu/drm/gma500/psb_device.c | 28 +++++++++++++++------- > drivers/gpu/drm/gma500/psb_drv.c | 10 ++++---- > drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++---- > 9 files changed, 84 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c > index f854f58bcbb3..dd32b484dd82 100644 > --- a/drivers/gpu/drm/gma500/cdv_device.c > +++ b/drivers/gpu/drm/gma500/cdv_device.c > @@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev) > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > struct pci_dev *pdev = to_pci_dev(dev->dev); > struct psb_save_area *regs = &dev_priv->regs; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > > dev_dbg(dev->dev, "Saving GPU registers.\n"); > @@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev) > regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R); > regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R); > > - list_for_each_entry(connector, &dev->mode_config.connector_list, head) > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) > connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF); > + drm_connector_list_iter_end(&conn_iter); > > return 0; > } > @@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev) > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > struct pci_dev *pdev = to_pci_dev(dev->dev); > struct psb_save_area *regs = &dev_priv->regs; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > u32 temp; > > @@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev) > > drm_mode_config_reset(dev); > > - list_for_each_entry(connector, &dev->mode_config.connector_list, head) > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) > connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); > + drm_connector_list_iter_end(&conn_iter); > > /* Resume the modeset for every activated CRTC */ > drm_helper_resume_force_mode(dev); > diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c > index 94ebc48a4349..0c3ddcdc29dc 100644 > --- a/drivers/gpu/drm/gma500/cdv_intel_display.c > +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c > @@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, > bool ok; > bool is_lvds = false; > bool is_dp = false; > - struct drm_mode_config *mode_config = &dev->mode_config; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > const struct gma_limit_t *limit; > u32 ddi_select = 0; > bool is_edp = false; > > - list_for_each_entry(connector, &mode_config->connector_list, head) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > struct gma_encoder *gma_encoder = > gma_attached_encoder(connector); > > @@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, > is_edp = true; > break; > default: > + drm_connector_list_iter_end(&conn_iter); > DRM_ERROR("invalid output type.\n"); > return 0; > } > + > + break; > } > + drm_connector_list_iter_end(&conn_iter); > > if (dev_priv->dplla_96mhz) > /* low-end sku, 96/100 mhz */ > diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c > index 2b99c996fdc2..0ac6ea5fd3a1 100644 > --- a/drivers/gpu/drm/gma500/framebuffer.c > +++ b/drivers/gpu/drm/gma500/framebuffer.c > @@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = { > static void psb_setup_outputs(struct drm_device *dev) > { > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > > drm_mode_create_scaling_mode_property(dev); > @@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev) > "backlight", 0, 100); > dev_priv->ops->output_init(dev); > > - list_for_each_entry(connector, &dev->mode_config.connector_list, > - head) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > struct gma_encoder *gma_encoder = gma_attached_encoder(connector); > struct drm_encoder *encoder = &gma_encoder->base; > int crtc_mask = 0, clone_mask = 0; > @@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev) > encoder->possible_clones = > gma_connector_clones(dev, clone_mask); > } > + drm_connector_list_iter_end(&conn_iter); > } > > void psb_modeset_init(struct drm_device *dev) > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c > index 1d7964c339f4..e8157464d9eb 100644 > --- a/drivers/gpu/drm/gma500/gma_display.c > +++ b/drivers/gpu/drm/gma500/gma_display.c > @@ -27,17 +27,21 @@ > bool gma_pipe_has_type(struct drm_crtc *crtc, int type) > { > struct drm_device *dev = crtc->dev; > - struct drm_mode_config *mode_config = &dev->mode_config; > - struct drm_connector *l_entry; > + struct drm_connector_list_iter conn_iter; > + struct drm_connector *connector; > > - list_for_each_entry(l_entry, &mode_config->connector_list, head) { > - if (l_entry->encoder && l_entry->encoder->crtc == crtc) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > + if (connector->encoder && connector->encoder->crtc == crtc) { > struct gma_encoder *gma_encoder = > - gma_attached_encoder(l_entry); > - if (gma_encoder->type == type) > + gma_attached_encoder(connector); > + if (gma_encoder->type == type) { > + drm_connector_list_iter_end(&conn_iter); > return true; > + } > } > } > + drm_connector_list_iter_end(&conn_iter); > > return false; > } > diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c > index 36c7c2686c90..873c17cf8fb4 100644 > --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c > +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c > @@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > bool ok, is_sdvo = false; > bool is_lvds = false; > bool is_mipi = false; > - struct drm_mode_config *mode_config = &dev->mode_config; > struct gma_encoder *gma_encoder = NULL; > uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > int i; > int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0; > @@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > adjusted_mode, > sizeof(struct drm_display_mode)); > > - list_for_each_entry(connector, &mode_config->connector_list, head) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > if (!connector->encoder || connector->encoder->crtc != crtc) > continue; > > @@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > is_mipi = true; > break; > } > + > + break; > } > > + if (gma_encoder) > + drm_object_property_get_value(&connector->base, > + dev->mode_config.scaling_mode_property, &scalingType); > + > + drm_connector_list_iter_end(&conn_iter); > + > /* Disable the VGA plane that we never use */ > for (i = 0; i <= need_aux; i++) > REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i); > @@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > (mode->crtc_vdisplay - 1), i); > } > > - if (gma_encoder) > - drm_object_property_get_value(&connector->base, > - dev->mode_config.scaling_mode_property, &scalingType); > - > if (scalingType == DRM_MODE_SCALE_NO_SCALE) { > /* Moorestown doesn't have register support for centering so > * we need to mess with the h/vblank and h/vsync start and > diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c > index 28b995ef2844..04852dbc7fb3 100644 > --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c > +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c > @@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, > struct drm_device *dev = encoder->dev; > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev; > - struct drm_mode_config *mode_config = &dev->mode_config; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector = NULL; > struct drm_crtc *crtc = encoder->crtc; > u32 lvds_port; > @@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, > REG_WRITE(LVDS, lvds_port); > > /* Find the connector we're trying to set up */ > - list_for_each_entry(connector, &mode_config->connector_list, head) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > if (connector->encoder && connector->encoder->crtc == crtc) > break; > } > > - if (list_entry_is_head(connector, &mode_config->connector_list, head)) { > + if (!connector) { > + drm_connector_list_iter_end(&conn_iter); > DRM_ERROR("Couldn't find connector when setting mode"); > gma_power_end(dev); > return; > } > > - drm_object_property_get_value( > - &connector->base, > - dev->mode_config.scaling_mode_property, > - &v); > + drm_object_property_get_value( &connector->base, > + dev->mode_config.scaling_mode_property, &v); > + drm_connector_list_iter_end(&conn_iter); > > if (v == DRM_MODE_SCALE_NO_SCALE) > REG_WRITE(PFIT_CONTROL, 0); > diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c > index 59f325165667..71534f4ca834 100644 > --- a/drivers/gpu/drm/gma500/psb_device.c > +++ b/drivers/gpu/drm/gma500/psb_device.c > @@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev) > static int psb_save_display_registers(struct drm_device *dev) > { > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > + struct gma_connector *gma_connector; > struct drm_crtc *crtc; > - struct gma_connector *connector; > + struct drm_connector_list_iter conn_iter; > + struct drm_connector *connector; > struct psb_state *regs = &dev_priv->regs.psb; > > /* Display arbitration control + watermarks */ > @@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev) > dev_priv->ops->save_crtc(crtc); > } > > - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) > - if (connector->save) > - connector->save(&connector->base); > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > + gma_connector = to_gma_connector(connector); > + if (gma_connector->save) > + gma_connector->save(connector); > + } > + drm_connector_list_iter_end(&conn_iter); > > drm_modeset_unlock_all(dev); > return 0; > @@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev) > static int psb_restore_display_registers(struct drm_device *dev) > { > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > + struct gma_connector *gma_connector; > struct drm_crtc *crtc; > - struct gma_connector *connector; > + struct drm_connector_list_iter conn_iter; > + struct drm_connector *connector; > struct psb_state *regs = &dev_priv->regs.psb; > > /* Display arbitration + watermarks */ > @@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev) > if (drm_helper_crtc_in_use(crtc)) > dev_priv->ops->restore_crtc(crtc); > > - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) > - if (connector->restore) > - connector->restore(&connector->base); > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > + gma_connector = to_gma_connector(connector); > + if (gma_connector->restore) > + gma_connector->restore(connector); > + } > + drm_connector_list_iter_end(&conn_iter); > > drm_modeset_unlock_all(dev); > return 0; > diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c > index b231fddb8817..bb0e3288e35b 100644 > --- a/drivers/gpu/drm/gma500/psb_drv.c > +++ b/drivers/gpu/drm/gma500/psb_drv.c > @@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > unsigned long resource_start, resource_len; > unsigned long irqflags; > - int ret = -ENOMEM; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > struct gma_encoder *gma_encoder; > struct psb_gtt *pg; > + int ret = -ENOMEM; > > /* initializing driver private data */ > > @@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) > psb_fbdev_init(dev); > drm_kms_helper_poll_init(dev); > > - /* Only add backlight support if we have LVDS output */ > - list_for_each_entry(connector, &dev->mode_config.connector_list, > - head) { > + /* Only add backlight support if we have LVDS or MIPI output */ > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > gma_encoder = gma_attached_encoder(connector); > > switch (gma_encoder->type) { > @@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) > break; > } > } > + drm_connector_list_iter_end(&conn_iter); > > if (ret) > return ret; > diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c > index a99859b5b13a..fb8234f4d128 100644 > --- a/drivers/gpu/drm/gma500/psb_intel_display.c > +++ b/drivers/gpu/drm/gma500/psb_intel_display.c > @@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, > u32 dpll = 0, fp = 0, dspcntr, pipeconf; > bool ok, is_sdvo = false; > bool is_lvds = false, is_tv = false; > - struct drm_mode_config *mode_config = &dev->mode_config; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > const struct gma_limit_t *limit; > > @@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, > return 0; > } > > - list_for_each_entry(connector, &mode_config->connector_list, head) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > struct gma_encoder *gma_encoder = gma_attached_encoder(connector); > > if (!connector->encoder > @@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, > break; > } > } > + drm_connector_list_iter_end(&conn_iter); > > refclk = 96000; > > @@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe) > > int gma_connector_clones(struct drm_device *dev, int type_mask) > { > - int index_mask = 0; > + struct drm_connector_list_iter conn_iter; > struct drm_connector *connector; > + int index_mask = 0; > int entry = 0; > > - list_for_each_entry(connector, &dev->mode_config.connector_list, > - head) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > struct gma_encoder *gma_encoder = gma_attached_encoder(connector); > if (type_mask & (1 << gma_encoder->type)) > index_mask |= (1 << entry); > entry++; > } > + drm_connector_list_iter_end(&conn_iter); > + > return index_mask; > } > -- > 2.35.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator 2022-03-22 14:39 ` Daniel Vetter @ 2022-03-22 15:46 ` Patrik Jakobsson 2022-03-22 19:34 ` Thomas Zimmermann 0 siblings, 1 reply; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 15:46 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, Sam Ravnborg, Thomas Zimmermann, dri-devel On Tue, Mar 22, 2022 at 3:39 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Tue, Mar 22, 2022 at 02:17:38PM +0100, Patrik Jakobsson wrote: > > This makes sure we're using proper locking when iterating the list of > > connectors. > > > > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > > Note that this is only needed if your driver deals with hotpluggable > connectors. Since gma500 doesn't, not really a need to convert this over, > but it also doesn't hurt. Good point. Not sure but I think there is a slight possibility that Cedarview can support DP MST. If someone adds support for DP MST then this code would make sense. I was never able to exercise the DP code because I couldn't find an eDP cable that fits the Intel DN2800MT (my only device with DP). Do you happen to know what the 40-pin connector is called? Perhaps Intel has some standard for eDP connectors? > > If the kerneldoc doesn't explain this, maybe we should add it? Care for a > patch. I didn't see it being mentioned anywhere. I'll send a patch. > > Either way on the entire series: > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Thanks for having a look. > > I'll leave it up to you whether you want to push this one too or not. > -Daniel > > > --- > > drivers/gpu/drm/gma500/cdv_device.c | 10 ++++++-- > > drivers/gpu/drm/gma500/cdv_intel_display.c | 9 +++++-- > > drivers/gpu/drm/gma500/framebuffer.c | 6 +++-- > > drivers/gpu/drm/gma500/gma_display.c | 16 ++++++++----- > > drivers/gpu/drm/gma500/oaktrail_crtc.c | 17 ++++++++----- > > drivers/gpu/drm/gma500/oaktrail_lvds.c | 15 ++++++------ > > drivers/gpu/drm/gma500/psb_device.c | 28 +++++++++++++++------- > > drivers/gpu/drm/gma500/psb_drv.c | 10 ++++---- > > drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++---- > > 9 files changed, 84 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c > > index f854f58bcbb3..dd32b484dd82 100644 > > --- a/drivers/gpu/drm/gma500/cdv_device.c > > +++ b/drivers/gpu/drm/gma500/cdv_device.c > > @@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev) > > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > > struct pci_dev *pdev = to_pci_dev(dev->dev); > > struct psb_save_area *regs = &dev_priv->regs; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > > > dev_dbg(dev->dev, "Saving GPU registers.\n"); > > @@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev) > > regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R); > > regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R); > > > > - list_for_each_entry(connector, &dev->mode_config.connector_list, head) > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) > > connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF); > > + drm_connector_list_iter_end(&conn_iter); > > > > return 0; > > } > > @@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev) > > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > > struct pci_dev *pdev = to_pci_dev(dev->dev); > > struct psb_save_area *regs = &dev_priv->regs; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > u32 temp; > > > > @@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev) > > > > drm_mode_config_reset(dev); > > > > - list_for_each_entry(connector, &dev->mode_config.connector_list, head) > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) > > connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); > > + drm_connector_list_iter_end(&conn_iter); > > > > /* Resume the modeset for every activated CRTC */ > > drm_helper_resume_force_mode(dev); > > diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c > > index 94ebc48a4349..0c3ddcdc29dc 100644 > > --- a/drivers/gpu/drm/gma500/cdv_intel_display.c > > +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c > > @@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, > > bool ok; > > bool is_lvds = false; > > bool is_dp = false; > > - struct drm_mode_config *mode_config = &dev->mode_config; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > const struct gma_limit_t *limit; > > u32 ddi_select = 0; > > bool is_edp = false; > > > > - list_for_each_entry(connector, &mode_config->connector_list, head) { > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > struct gma_encoder *gma_encoder = > > gma_attached_encoder(connector); > > > > @@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, > > is_edp = true; > > break; > > default: > > + drm_connector_list_iter_end(&conn_iter); > > DRM_ERROR("invalid output type.\n"); > > return 0; > > } > > + > > + break; > > } > > + drm_connector_list_iter_end(&conn_iter); > > > > if (dev_priv->dplla_96mhz) > > /* low-end sku, 96/100 mhz */ > > diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c > > index 2b99c996fdc2..0ac6ea5fd3a1 100644 > > --- a/drivers/gpu/drm/gma500/framebuffer.c > > +++ b/drivers/gpu/drm/gma500/framebuffer.c > > @@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = { > > static void psb_setup_outputs(struct drm_device *dev) > > { > > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > > > drm_mode_create_scaling_mode_property(dev); > > @@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev) > > "backlight", 0, 100); > > dev_priv->ops->output_init(dev); > > > > - list_for_each_entry(connector, &dev->mode_config.connector_list, > > - head) { > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > struct gma_encoder *gma_encoder = gma_attached_encoder(connector); > > struct drm_encoder *encoder = &gma_encoder->base; > > int crtc_mask = 0, clone_mask = 0; > > @@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev) > > encoder->possible_clones = > > gma_connector_clones(dev, clone_mask); > > } > > + drm_connector_list_iter_end(&conn_iter); > > } > > > > void psb_modeset_init(struct drm_device *dev) > > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c > > index 1d7964c339f4..e8157464d9eb 100644 > > --- a/drivers/gpu/drm/gma500/gma_display.c > > +++ b/drivers/gpu/drm/gma500/gma_display.c > > @@ -27,17 +27,21 @@ > > bool gma_pipe_has_type(struct drm_crtc *crtc, int type) > > { > > struct drm_device *dev = crtc->dev; > > - struct drm_mode_config *mode_config = &dev->mode_config; > > - struct drm_connector *l_entry; > > + struct drm_connector_list_iter conn_iter; > > + struct drm_connector *connector; > > > > - list_for_each_entry(l_entry, &mode_config->connector_list, head) { > > - if (l_entry->encoder && l_entry->encoder->crtc == crtc) { > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > + if (connector->encoder && connector->encoder->crtc == crtc) { > > struct gma_encoder *gma_encoder = > > - gma_attached_encoder(l_entry); > > - if (gma_encoder->type == type) > > + gma_attached_encoder(connector); > > + if (gma_encoder->type == type) { > > + drm_connector_list_iter_end(&conn_iter); > > return true; > > + } > > } > > } > > + drm_connector_list_iter_end(&conn_iter); > > > > return false; > > } > > diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c > > index 36c7c2686c90..873c17cf8fb4 100644 > > --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c > > +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c > > @@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > > bool ok, is_sdvo = false; > > bool is_lvds = false; > > bool is_mipi = false; > > - struct drm_mode_config *mode_config = &dev->mode_config; > > struct gma_encoder *gma_encoder = NULL; > > uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > int i; > > int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0; > > @@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > > adjusted_mode, > > sizeof(struct drm_display_mode)); > > > > - list_for_each_entry(connector, &mode_config->connector_list, head) { > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > if (!connector->encoder || connector->encoder->crtc != crtc) > > continue; > > > > @@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > > is_mipi = true; > > break; > > } > > + > > + break; > > } > > > > + if (gma_encoder) > > + drm_object_property_get_value(&connector->base, > > + dev->mode_config.scaling_mode_property, &scalingType); > > + > > + drm_connector_list_iter_end(&conn_iter); > > + > > /* Disable the VGA plane that we never use */ > > for (i = 0; i <= need_aux; i++) > > REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i); > > @@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, > > (mode->crtc_vdisplay - 1), i); > > } > > > > - if (gma_encoder) > > - drm_object_property_get_value(&connector->base, > > - dev->mode_config.scaling_mode_property, &scalingType); > > - > > if (scalingType == DRM_MODE_SCALE_NO_SCALE) { > > /* Moorestown doesn't have register support for centering so > > * we need to mess with the h/vblank and h/vsync start and > > diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c > > index 28b995ef2844..04852dbc7fb3 100644 > > --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c > > +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c > > @@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, > > struct drm_device *dev = encoder->dev; > > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > > struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev; > > - struct drm_mode_config *mode_config = &dev->mode_config; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector = NULL; > > struct drm_crtc *crtc = encoder->crtc; > > u32 lvds_port; > > @@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, > > REG_WRITE(LVDS, lvds_port); > > > > /* Find the connector we're trying to set up */ > > - list_for_each_entry(connector, &mode_config->connector_list, head) { > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > if (connector->encoder && connector->encoder->crtc == crtc) > > break; > > } > > > > - if (list_entry_is_head(connector, &mode_config->connector_list, head)) { > > + if (!connector) { > > + drm_connector_list_iter_end(&conn_iter); > > DRM_ERROR("Couldn't find connector when setting mode"); > > gma_power_end(dev); > > return; > > } > > > > - drm_object_property_get_value( > > - &connector->base, > > - dev->mode_config.scaling_mode_property, > > - &v); > > + drm_object_property_get_value( &connector->base, > > + dev->mode_config.scaling_mode_property, &v); > > + drm_connector_list_iter_end(&conn_iter); > > > > if (v == DRM_MODE_SCALE_NO_SCALE) > > REG_WRITE(PFIT_CONTROL, 0); > > diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c > > index 59f325165667..71534f4ca834 100644 > > --- a/drivers/gpu/drm/gma500/psb_device.c > > +++ b/drivers/gpu/drm/gma500/psb_device.c > > @@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev) > > static int psb_save_display_registers(struct drm_device *dev) > > { > > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > > + struct gma_connector *gma_connector; > > struct drm_crtc *crtc; > > - struct gma_connector *connector; > > + struct drm_connector_list_iter conn_iter; > > + struct drm_connector *connector; > > struct psb_state *regs = &dev_priv->regs.psb; > > > > /* Display arbitration control + watermarks */ > > @@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev) > > dev_priv->ops->save_crtc(crtc); > > } > > > > - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) > > - if (connector->save) > > - connector->save(&connector->base); > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > + gma_connector = to_gma_connector(connector); > > + if (gma_connector->save) > > + gma_connector->save(connector); > > + } > > + drm_connector_list_iter_end(&conn_iter); > > > > drm_modeset_unlock_all(dev); > > return 0; > > @@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev) > > static int psb_restore_display_registers(struct drm_device *dev) > > { > > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > > + struct gma_connector *gma_connector; > > struct drm_crtc *crtc; > > - struct gma_connector *connector; > > + struct drm_connector_list_iter conn_iter; > > + struct drm_connector *connector; > > struct psb_state *regs = &dev_priv->regs.psb; > > > > /* Display arbitration + watermarks */ > > @@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev) > > if (drm_helper_crtc_in_use(crtc)) > > dev_priv->ops->restore_crtc(crtc); > > > > - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) > > - if (connector->restore) > > - connector->restore(&connector->base); > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > + gma_connector = to_gma_connector(connector); > > + if (gma_connector->restore) > > + gma_connector->restore(connector); > > + } > > + drm_connector_list_iter_end(&conn_iter); > > > > drm_modeset_unlock_all(dev); > > return 0; > > diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c > > index b231fddb8817..bb0e3288e35b 100644 > > --- a/drivers/gpu/drm/gma500/psb_drv.c > > +++ b/drivers/gpu/drm/gma500/psb_drv.c > > @@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) > > struct drm_psb_private *dev_priv = to_drm_psb_private(dev); > > unsigned long resource_start, resource_len; > > unsigned long irqflags; > > - int ret = -ENOMEM; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > struct gma_encoder *gma_encoder; > > struct psb_gtt *pg; > > + int ret = -ENOMEM; > > > > /* initializing driver private data */ > > > > @@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) > > psb_fbdev_init(dev); > > drm_kms_helper_poll_init(dev); > > > > - /* Only add backlight support if we have LVDS output */ > > - list_for_each_entry(connector, &dev->mode_config.connector_list, > > - head) { > > + /* Only add backlight support if we have LVDS or MIPI output */ > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > gma_encoder = gma_attached_encoder(connector); > > > > switch (gma_encoder->type) { > > @@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) > > break; > > } > > } > > + drm_connector_list_iter_end(&conn_iter); > > > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c > > index a99859b5b13a..fb8234f4d128 100644 > > --- a/drivers/gpu/drm/gma500/psb_intel_display.c > > +++ b/drivers/gpu/drm/gma500/psb_intel_display.c > > @@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, > > u32 dpll = 0, fp = 0, dspcntr, pipeconf; > > bool ok, is_sdvo = false; > > bool is_lvds = false, is_tv = false; > > - struct drm_mode_config *mode_config = &dev->mode_config; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > const struct gma_limit_t *limit; > > > > @@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, > > return 0; > > } > > > > - list_for_each_entry(connector, &mode_config->connector_list, head) { > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > struct gma_encoder *gma_encoder = gma_attached_encoder(connector); > > > > if (!connector->encoder > > @@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, > > break; > > } > > } > > + drm_connector_list_iter_end(&conn_iter); > > > > refclk = 96000; > > > > @@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe) > > > > int gma_connector_clones(struct drm_device *dev, int type_mask) > > { > > - int index_mask = 0; > > + struct drm_connector_list_iter conn_iter; > > struct drm_connector *connector; > > + int index_mask = 0; > > int entry = 0; > > > > - list_for_each_entry(connector, &dev->mode_config.connector_list, > > - head) { > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(connector, &conn_iter) { > > struct gma_encoder *gma_encoder = gma_attached_encoder(connector); > > if (type_mask & (1 << gma_encoder->type)) > > index_mask |= (1 << entry); > > entry++; > > } > > + drm_connector_list_iter_end(&conn_iter); > > + > > return index_mask; > > } > > -- > > 2.35.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator 2022-03-22 15:46 ` Patrik Jakobsson @ 2022-03-22 19:34 ` Thomas Zimmermann 0 siblings, 0 replies; 15+ messages in thread From: Thomas Zimmermann @ 2022-03-22 19:34 UTC (permalink / raw) To: Patrik Jakobsson, Daniel Vetter; +Cc: Daniel Vetter, Sam Ravnborg, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 21226 bytes --] Hi Am 22.03.22 um 16:46 schrieb Patrik Jakobsson: > On Tue, Mar 22, 2022 at 3:39 PM Daniel Vetter <daniel@ffwll.ch> wrote: >> >> On Tue, Mar 22, 2022 at 02:17:38PM +0100, Patrik Jakobsson wrote: >>> This makes sure we're using proper locking when iterating the list of >>> connectors. >>> >>> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >> >> Note that this is only needed if your driver deals with hotpluggable >> connectors. Since gma500 doesn't, not really a need to convert this over, >> but it also doesn't hurt. I'd fix this if only for copy-pasters to not copy incorrect code. :) > > Good point. Not sure but I think there is a slight possibility that > Cedarview can support DP MST. If someone adds support for DP MST then > this code would make sense. I was never able to exercise the DP code > because I couldn't find an eDP cable that fits the Intel DN2800MT (my > only device with DP). Do you happen to know what the 40-pin connector > is called? Perhaps Intel has some standard for eDP connectors? > >> >> If the kerneldoc doesn't explain this, maybe we should add it? Care for a >> patch. > > I didn't see it being mentioned anywhere. I'll send a patch. > >> >> Either way on the entire series: >> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Thanks for having a look. > >> >> I'll leave it up to you whether you want to push this one too or not. >> -Daniel >> >>> --- >>> drivers/gpu/drm/gma500/cdv_device.c | 10 ++++++-- >>> drivers/gpu/drm/gma500/cdv_intel_display.c | 9 +++++-- >>> drivers/gpu/drm/gma500/framebuffer.c | 6 +++-- >>> drivers/gpu/drm/gma500/gma_display.c | 16 ++++++++----- >>> drivers/gpu/drm/gma500/oaktrail_crtc.c | 17 ++++++++----- >>> drivers/gpu/drm/gma500/oaktrail_lvds.c | 15 ++++++------ >>> drivers/gpu/drm/gma500/psb_device.c | 28 +++++++++++++++------- >>> drivers/gpu/drm/gma500/psb_drv.c | 10 ++++---- >>> drivers/gpu/drm/gma500/psb_intel_display.c | 15 ++++++++---- >>> 9 files changed, 84 insertions(+), 42 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c >>> index f854f58bcbb3..dd32b484dd82 100644 >>> --- a/drivers/gpu/drm/gma500/cdv_device.c >>> +++ b/drivers/gpu/drm/gma500/cdv_device.c >>> @@ -262,6 +262,7 @@ static int cdv_save_display_registers(struct drm_device *dev) >>> struct drm_psb_private *dev_priv = to_drm_psb_private(dev); >>> struct pci_dev *pdev = to_pci_dev(dev->dev); >>> struct psb_save_area *regs = &dev_priv->regs; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> >>> dev_dbg(dev->dev, "Saving GPU registers.\n"); >>> @@ -298,8 +299,10 @@ static int cdv_save_display_registers(struct drm_device *dev) >>> regs->cdv.saveIER = REG_READ(PSB_INT_ENABLE_R); >>> regs->cdv.saveIMR = REG_READ(PSB_INT_MASK_R); >>> >>> - list_for_each_entry(connector, &dev->mode_config.connector_list, head) >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) >>> connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF); >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> return 0; >>> } >>> @@ -317,6 +320,7 @@ static int cdv_restore_display_registers(struct drm_device *dev) >>> struct drm_psb_private *dev_priv = to_drm_psb_private(dev); >>> struct pci_dev *pdev = to_pci_dev(dev->dev); >>> struct psb_save_area *regs = &dev_priv->regs; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> u32 temp; >>> >>> @@ -373,8 +377,10 @@ static int cdv_restore_display_registers(struct drm_device *dev) >>> >>> drm_mode_config_reset(dev); >>> >>> - list_for_each_entry(connector, &dev->mode_config.connector_list, head) >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) >>> connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> /* Resume the modeset for every activated CRTC */ >>> drm_helper_resume_force_mode(dev); >>> diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c >>> index 94ebc48a4349..0c3ddcdc29dc 100644 >>> --- a/drivers/gpu/drm/gma500/cdv_intel_display.c >>> +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c >>> @@ -584,13 +584,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, >>> bool ok; >>> bool is_lvds = false; >>> bool is_dp = false; >>> - struct drm_mode_config *mode_config = &dev->mode_config; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> const struct gma_limit_t *limit; >>> u32 ddi_select = 0; >>> bool is_edp = false; >>> >>> - list_for_each_entry(connector, &mode_config->connector_list, head) { >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> struct gma_encoder *gma_encoder = >>> gma_attached_encoder(connector); >>> >>> @@ -613,10 +614,14 @@ static int cdv_intel_crtc_mode_set(struct drm_crtc *crtc, >>> is_edp = true; >>> break; >>> default: >>> + drm_connector_list_iter_end(&conn_iter); >>> DRM_ERROR("invalid output type.\n"); >>> return 0; >>> } >>> + >>> + break; >>> } >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> if (dev_priv->dplla_96mhz) >>> /* low-end sku, 96/100 mhz */ >>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c >>> index 2b99c996fdc2..0ac6ea5fd3a1 100644 >>> --- a/drivers/gpu/drm/gma500/framebuffer.c >>> +++ b/drivers/gpu/drm/gma500/framebuffer.c >>> @@ -451,6 +451,7 @@ static const struct drm_mode_config_funcs psb_mode_funcs = { >>> static void psb_setup_outputs(struct drm_device *dev) >>> { >>> struct drm_psb_private *dev_priv = to_drm_psb_private(dev); >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> >>> drm_mode_create_scaling_mode_property(dev); >>> @@ -461,8 +462,8 @@ static void psb_setup_outputs(struct drm_device *dev) >>> "backlight", 0, 100); >>> dev_priv->ops->output_init(dev); >>> >>> - list_for_each_entry(connector, &dev->mode_config.connector_list, >>> - head) { >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> struct gma_encoder *gma_encoder = gma_attached_encoder(connector); >>> struct drm_encoder *encoder = &gma_encoder->base; >>> int crtc_mask = 0, clone_mask = 0; >>> @@ -505,6 +506,7 @@ static void psb_setup_outputs(struct drm_device *dev) >>> encoder->possible_clones = >>> gma_connector_clones(dev, clone_mask); >>> } >>> + drm_connector_list_iter_end(&conn_iter); >>> } >>> >>> void psb_modeset_init(struct drm_device *dev) >>> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c >>> index 1d7964c339f4..e8157464d9eb 100644 >>> --- a/drivers/gpu/drm/gma500/gma_display.c >>> +++ b/drivers/gpu/drm/gma500/gma_display.c >>> @@ -27,17 +27,21 @@ >>> bool gma_pipe_has_type(struct drm_crtc *crtc, int type) >>> { >>> struct drm_device *dev = crtc->dev; >>> - struct drm_mode_config *mode_config = &dev->mode_config; >>> - struct drm_connector *l_entry; >>> + struct drm_connector_list_iter conn_iter; >>> + struct drm_connector *connector; >>> >>> - list_for_each_entry(l_entry, &mode_config->connector_list, head) { >>> - if (l_entry->encoder && l_entry->encoder->crtc == crtc) { >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> + if (connector->encoder && connector->encoder->crtc == crtc) { >>> struct gma_encoder *gma_encoder = >>> - gma_attached_encoder(l_entry); >>> - if (gma_encoder->type == type) >>> + gma_attached_encoder(connector); >>> + if (gma_encoder->type == type) { >>> + drm_connector_list_iter_end(&conn_iter); >>> return true; >>> + } >>> } >>> } >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> return false; >>> } >>> diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c >>> index 36c7c2686c90..873c17cf8fb4 100644 >>> --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c >>> +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c >>> @@ -372,9 +372,9 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, >>> bool ok, is_sdvo = false; >>> bool is_lvds = false; >>> bool is_mipi = false; >>> - struct drm_mode_config *mode_config = &dev->mode_config; >>> struct gma_encoder *gma_encoder = NULL; >>> uint64_t scalingType = DRM_MODE_SCALE_FULLSCREEN; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> int i; >>> int need_aux = gma_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ? 1 : 0; >>> @@ -392,7 +392,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, >>> adjusted_mode, >>> sizeof(struct drm_display_mode)); >>> >>> - list_for_each_entry(connector, &mode_config->connector_list, head) { >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> if (!connector->encoder || connector->encoder->crtc != crtc) >>> continue; >>> >>> @@ -409,8 +410,16 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, >>> is_mipi = true; >>> break; >>> } >>> + >>> + break; >>> } >>> >>> + if (gma_encoder) >>> + drm_object_property_get_value(&connector->base, >>> + dev->mode_config.scaling_mode_property, &scalingType); >>> + >>> + drm_connector_list_iter_end(&conn_iter); >>> + >>> /* Disable the VGA plane that we never use */ >>> for (i = 0; i <= need_aux; i++) >>> REG_WRITE_WITH_AUX(VGACNTRL, VGA_DISP_DISABLE, i); >>> @@ -424,10 +433,6 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, >>> (mode->crtc_vdisplay - 1), i); >>> } >>> >>> - if (gma_encoder) >>> - drm_object_property_get_value(&connector->base, >>> - dev->mode_config.scaling_mode_property, &scalingType); >>> - >>> if (scalingType == DRM_MODE_SCALE_NO_SCALE) { >>> /* Moorestown doesn't have register support for centering so >>> * we need to mess with the h/vblank and h/vsync start and >>> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c >>> index 28b995ef2844..04852dbc7fb3 100644 >>> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c >>> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c >>> @@ -85,7 +85,7 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, >>> struct drm_device *dev = encoder->dev; >>> struct drm_psb_private *dev_priv = to_drm_psb_private(dev); >>> struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev; >>> - struct drm_mode_config *mode_config = &dev->mode_config; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector = NULL; >>> struct drm_crtc *crtc = encoder->crtc; >>> u32 lvds_port; >>> @@ -112,21 +112,22 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder, >>> REG_WRITE(LVDS, lvds_port); >>> >>> /* Find the connector we're trying to set up */ >>> - list_for_each_entry(connector, &mode_config->connector_list, head) { >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> if (connector->encoder && connector->encoder->crtc == crtc) >>> break; >>> } >>> >>> - if (list_entry_is_head(connector, &mode_config->connector_list, head)) { >>> + if (!connector) { >>> + drm_connector_list_iter_end(&conn_iter); >>> DRM_ERROR("Couldn't find connector when setting mode"); >>> gma_power_end(dev); >>> return; >>> } >>> >>> - drm_object_property_get_value( >>> - &connector->base, >>> - dev->mode_config.scaling_mode_property, >>> - &v); >>> + drm_object_property_get_value( &connector->base, >>> + dev->mode_config.scaling_mode_property, &v); >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> if (v == DRM_MODE_SCALE_NO_SCALE) >>> REG_WRITE(PFIT_CONTROL, 0); >>> diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c >>> index 59f325165667..71534f4ca834 100644 >>> --- a/drivers/gpu/drm/gma500/psb_device.c >>> +++ b/drivers/gpu/drm/gma500/psb_device.c >>> @@ -168,8 +168,10 @@ static void psb_init_pm(struct drm_device *dev) >>> static int psb_save_display_registers(struct drm_device *dev) >>> { >>> struct drm_psb_private *dev_priv = to_drm_psb_private(dev); >>> + struct gma_connector *gma_connector; >>> struct drm_crtc *crtc; >>> - struct gma_connector *connector; >>> + struct drm_connector_list_iter conn_iter; >>> + struct drm_connector *connector; >>> struct psb_state *regs = &dev_priv->regs.psb; >>> >>> /* Display arbitration control + watermarks */ >>> @@ -189,9 +191,13 @@ static int psb_save_display_registers(struct drm_device *dev) >>> dev_priv->ops->save_crtc(crtc); >>> } >>> >>> - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) >>> - if (connector->save) >>> - connector->save(&connector->base); >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> + gma_connector = to_gma_connector(connector); >>> + if (gma_connector->save) >>> + gma_connector->save(connector); >>> + } >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> drm_modeset_unlock_all(dev); >>> return 0; >>> @@ -206,8 +212,10 @@ static int psb_save_display_registers(struct drm_device *dev) >>> static int psb_restore_display_registers(struct drm_device *dev) >>> { >>> struct drm_psb_private *dev_priv = to_drm_psb_private(dev); >>> + struct gma_connector *gma_connector; >>> struct drm_crtc *crtc; >>> - struct gma_connector *connector; >>> + struct drm_connector_list_iter conn_iter; >>> + struct drm_connector *connector; >>> struct psb_state *regs = &dev_priv->regs.psb; >>> >>> /* Display arbitration + watermarks */ >>> @@ -228,9 +236,13 @@ static int psb_restore_display_registers(struct drm_device *dev) >>> if (drm_helper_crtc_in_use(crtc)) >>> dev_priv->ops->restore_crtc(crtc); >>> >>> - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) >>> - if (connector->restore) >>> - connector->restore(&connector->base); >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> + gma_connector = to_gma_connector(connector); >>> + if (gma_connector->restore) >>> + gma_connector->restore(connector); >>> + } >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> drm_modeset_unlock_all(dev); >>> return 0; >>> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c >>> index b231fddb8817..bb0e3288e35b 100644 >>> --- a/drivers/gpu/drm/gma500/psb_drv.c >>> +++ b/drivers/gpu/drm/gma500/psb_drv.c >>> @@ -236,10 +236,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) >>> struct drm_psb_private *dev_priv = to_drm_psb_private(dev); >>> unsigned long resource_start, resource_len; >>> unsigned long irqflags; >>> - int ret = -ENOMEM; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> struct gma_encoder *gma_encoder; >>> struct psb_gtt *pg; >>> + int ret = -ENOMEM; >>> >>> /* initializing driver private data */ >>> >>> @@ -390,9 +391,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) >>> psb_fbdev_init(dev); >>> drm_kms_helper_poll_init(dev); >>> >>> - /* Only add backlight support if we have LVDS output */ >>> - list_for_each_entry(connector, &dev->mode_config.connector_list, >>> - head) { >>> + /* Only add backlight support if we have LVDS or MIPI output */ >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> gma_encoder = gma_attached_encoder(connector); >>> >>> switch (gma_encoder->type) { >>> @@ -402,6 +403,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags) >>> break; >>> } >>> } >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> if (ret) >>> return ret; >>> diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c >>> index a99859b5b13a..fb8234f4d128 100644 >>> --- a/drivers/gpu/drm/gma500/psb_intel_display.c >>> +++ b/drivers/gpu/drm/gma500/psb_intel_display.c >>> @@ -106,7 +106,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, >>> u32 dpll = 0, fp = 0, dspcntr, pipeconf; >>> bool ok, is_sdvo = false; >>> bool is_lvds = false, is_tv = false; >>> - struct drm_mode_config *mode_config = &dev->mode_config; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> const struct gma_limit_t *limit; >>> >>> @@ -116,7 +116,8 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, >>> return 0; >>> } >>> >>> - list_for_each_entry(connector, &mode_config->connector_list, head) { >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> struct gma_encoder *gma_encoder = gma_attached_encoder(connector); >>> >>> if (!connector->encoder >>> @@ -135,6 +136,7 @@ static int psb_intel_crtc_mode_set(struct drm_crtc *crtc, >>> break; >>> } >>> } >>> + drm_connector_list_iter_end(&conn_iter); >>> >>> refclk = 96000; >>> >>> @@ -534,16 +536,19 @@ struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe) >>> >>> int gma_connector_clones(struct drm_device *dev, int type_mask) >>> { >>> - int index_mask = 0; >>> + struct drm_connector_list_iter conn_iter; >>> struct drm_connector *connector; >>> + int index_mask = 0; >>> int entry = 0; >>> >>> - list_for_each_entry(connector, &dev->mode_config.connector_list, >>> - head) { >>> + drm_connector_list_iter_begin(dev, &conn_iter); >>> + drm_for_each_connector_iter(connector, &conn_iter) { >>> struct gma_encoder *gma_encoder = gma_attached_encoder(connector); >>> if (type_mask & (1 << gma_encoder->type)) >>> index_mask |= (1 << entry); >>> entry++; >>> } >>> + drm_connector_list_iter_end(&conn_iter); >>> + >>> return index_mask; >>> } >>> -- >>> 2.35.1 >>> >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/7] drm/gma500: gma500 don't register non-hotpluggable connectors 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 2/7] drm/gma500: Uninstall interrupts on driver removal Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 3/7] drm/gma500: Make use of the drm connector iterator Patrik Jakobsson @ 2022-03-22 13:17 ` Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 5/7] drm/gma500: Free the correct connector allocation Patrik Jakobsson ` (3 subsequent siblings) 6 siblings, 0 replies; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 13:17 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, sam, tzimmermann According to docs we should only register connectors that are hotpluggable. No connectors in gma500 are hotpluggable. Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> --- drivers/gpu/drm/gma500/cdv_intel_crt.c | 3 --- drivers/gpu/drm/gma500/cdv_intel_dp.c | 3 --- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 2 -- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 2 -- drivers/gpu/drm/gma500/oaktrail_hdmi.c | 1 - drivers/gpu/drm/gma500/oaktrail_lvds.c | 1 - drivers/gpu/drm/gma500/psb_intel_lvds.c | 2 -- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 2 -- 8 files changed, 16 deletions(-) diff --git a/drivers/gpu/drm/gma500/cdv_intel_crt.c b/drivers/gpu/drm/gma500/cdv_intel_crt.c index 4a9bb4994a26..1ae0fbbda0eb 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_crt.c +++ b/drivers/gpu/drm/gma500/cdv_intel_crt.c @@ -194,7 +194,6 @@ static void cdv_intel_crt_destroy(struct drm_connector *connector) struct gma_encoder *gma_encoder = gma_attached_encoder(connector); psb_intel_i2c_destroy(gma_encoder->ddc_bus); - drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); } @@ -281,8 +280,6 @@ void cdv_intel_crt_init(struct drm_device *dev, drm_connector_helper_add(connector, &cdv_intel_crt_connector_helper_funcs); - drm_connector_register(connector); - return; failed_ddc: drm_encoder_cleanup(&gma_encoder->base); diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c index f562e91337c7..3fba9d4e785d 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c @@ -1866,7 +1866,6 @@ cdv_intel_dp_destroy(struct drm_connector *connector) intel_dp->panel_fixed_mode = NULL; } i2c_del_adapter(&intel_dp->adapter); - drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); } @@ -1990,8 +1989,6 @@ cdv_intel_dp_init(struct drm_device *dev, struct psb_intel_mode_device *mode_dev connector->interlace_allowed = false; connector->doublescan_allowed = false; - drm_connector_register(connector); - /* Set up the DDC bus. */ switch (output_reg) { case DP_B: diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c index e525689f84f0..e0d4c49b3c92 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c @@ -245,7 +245,6 @@ static void cdv_hdmi_destroy(struct drm_connector *connector) struct gma_encoder *gma_encoder = gma_attached_encoder(connector); psb_intel_i2c_destroy(gma_encoder->i2c_bus); - drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); } @@ -352,7 +351,6 @@ void cdv_hdmi_init(struct drm_device *dev, hdmi_priv->hdmi_i2c_adapter = &(gma_encoder->i2c_bus->adapter); hdmi_priv->dev = dev; - drm_connector_register(connector); return; failed_ddc: diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c index 9e1cdb11023c..851a3cc4653e 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c @@ -329,7 +329,6 @@ static void cdv_intel_lvds_destroy(struct drm_connector *connector) struct gma_encoder *gma_encoder = gma_attached_encoder(connector); psb_intel_i2c_destroy(gma_encoder->i2c_bus); - drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); } @@ -647,7 +646,6 @@ void cdv_intel_lvds_init(struct drm_device *dev, out: mutex_unlock(&dev->mode_config.mutex); - drm_connector_register(connector); return; failed_find: diff --git a/drivers/gpu/drm/gma500/oaktrail_hdmi.c b/drivers/gpu/drm/gma500/oaktrail_hdmi.c index 6eef60a5ac27..b5946a1cdcd5 100644 --- a/drivers/gpu/drm/gma500/oaktrail_hdmi.c +++ b/drivers/gpu/drm/gma500/oaktrail_hdmi.c @@ -654,7 +654,6 @@ void oaktrail_hdmi_init(struct drm_device *dev, connector->display_info.subpixel_order = SubPixelHorizontalRGB; connector->interlace_allowed = false; connector->doublescan_allowed = false; - drm_connector_register(connector); dev_info(dev->dev, "HDMI initialised.\n"); return; diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c index 04852dbc7fb3..aed5de8f8245 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c @@ -401,7 +401,6 @@ void oaktrail_lvds_init(struct drm_device *dev, out: mutex_unlock(&dev->mode_config.mutex); - drm_connector_register(connector); return; failed_find: diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index ac97e0d3c7dd..ec8f0b504ccc 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -525,7 +525,6 @@ void psb_intel_lvds_destroy(struct drm_connector *connector) struct psb_intel_lvds_priv *lvds_priv = gma_encoder->dev_priv; psb_intel_i2c_destroy(lvds_priv->ddc_bus); - drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); } @@ -782,7 +781,6 @@ void psb_intel_lvds_init(struct drm_device *dev, */ out: mutex_unlock(&dev->mode_config.mutex); - drm_connector_register(connector); return; failed_find: diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c index 042c4392e676..5b72a759a182 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -1542,7 +1542,6 @@ static int psb_intel_sdvo_get_modes(struct drm_connector *connector) static void psb_intel_sdvo_destroy(struct drm_connector *connector) { - drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); } @@ -1932,7 +1931,6 @@ psb_intel_sdvo_connector_init(struct psb_intel_sdvo_connector *connector, connector->base.restore = psb_intel_sdvo_restore; gma_connector_attach_encoder(&connector->base, &encoder->base); - drm_connector_register(&connector->base.base); } static void -- 2.35.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/7] drm/gma500: Free the correct connector allocation 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson ` (2 preceding siblings ...) 2022-03-22 13:17 ` [PATCH 4/7] drm/gma500: gma500 don't register non-hotpluggable connectors Patrik Jakobsson @ 2022-03-22 13:17 ` Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 6/7] drm/gma500: Declare a few functions static Patrik Jakobsson ` (2 subsequent siblings) 6 siblings, 0 replies; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 13:17 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, sam, tzimmermann The allocation is made for the gma_connector object so we must use the same address when free()ing the object. Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> --- drivers/gpu/drm/gma500/cdv_intel_crt.c | 3 ++- drivers/gpu/drm/gma500/cdv_intel_dp.c | 3 ++- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 3 ++- drivers/gpu/drm/gma500/psb_intel_lvds.c | 3 ++- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 4 +++- 6 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/gma500/cdv_intel_crt.c b/drivers/gpu/drm/gma500/cdv_intel_crt.c index 1ae0fbbda0eb..6bcd18c63c31 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_crt.c +++ b/drivers/gpu/drm/gma500/cdv_intel_crt.c @@ -191,11 +191,12 @@ static enum drm_connector_status cdv_intel_crt_detect( static void cdv_intel_crt_destroy(struct drm_connector *connector) { + struct gma_connector *gma_connector = to_gma_connector(connector); struct gma_encoder *gma_encoder = gma_attached_encoder(connector); psb_intel_i2c_destroy(gma_encoder->ddc_bus); drm_connector_cleanup(connector); - kfree(connector); + kfree(gma_connector); } static int cdv_intel_crt_get_modes(struct drm_connector *connector) diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c index 3fba9d4e785d..72b1b2fc3c27 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c @@ -1857,6 +1857,7 @@ cdv_intel_dp_set_property(struct drm_connector *connector, static void cdv_intel_dp_destroy(struct drm_connector *connector) { + struct gma_connector *gma_connector = to_gma_connector(connector); struct gma_encoder *gma_encoder = gma_attached_encoder(connector); struct cdv_intel_dp *intel_dp = gma_encoder->dev_priv; @@ -1867,7 +1868,7 @@ cdv_intel_dp_destroy(struct drm_connector *connector) } i2c_del_adapter(&intel_dp->adapter); drm_connector_cleanup(connector); - kfree(connector); + kfree(gma_connector); } static const struct drm_encoder_helper_funcs cdv_intel_dp_helper_funcs = { diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c index e0d4c49b3c92..8987e555e113 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c @@ -242,11 +242,12 @@ static enum drm_mode_status cdv_hdmi_mode_valid(struct drm_connector *connector, static void cdv_hdmi_destroy(struct drm_connector *connector) { + struct gma_connector *gma_connector = to_gma_connector(connector); struct gma_encoder *gma_encoder = gma_attached_encoder(connector); psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_cleanup(connector); - kfree(connector); + kfree(gma_connector); } static const struct drm_encoder_helper_funcs cdv_hdmi_helper_funcs = { diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c index 851a3cc4653e..98d9f5483a7c 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c @@ -326,11 +326,12 @@ static int cdv_intel_lvds_get_modes(struct drm_connector *connector) */ static void cdv_intel_lvds_destroy(struct drm_connector *connector) { + struct gma_connector *gma_connector = to_gma_connector(connector); struct gma_encoder *gma_encoder = gma_attached_encoder(connector); psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_cleanup(connector); - kfree(connector); + kfree(gma_connector); } static int cdv_intel_lvds_set_property(struct drm_connector *connector, diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index ec8f0b504ccc..cad00380b386 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -521,12 +521,13 @@ static int psb_intel_lvds_get_modes(struct drm_connector *connector) */ void psb_intel_lvds_destroy(struct drm_connector *connector) { + struct gma_connector *gma_connector = to_gma_connector(connector); struct gma_encoder *gma_encoder = gma_attached_encoder(connector); struct psb_intel_lvds_priv *lvds_priv = gma_encoder->dev_priv; psb_intel_i2c_destroy(lvds_priv->ddc_bus); drm_connector_cleanup(connector); - kfree(connector); + kfree(gma_connector); } int psb_intel_lvds_set_property(struct drm_connector *connector, diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c index 5b72a759a182..a85aace25548 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -1542,8 +1542,10 @@ static int psb_intel_sdvo_get_modes(struct drm_connector *connector) static void psb_intel_sdvo_destroy(struct drm_connector *connector) { + struct gma_connector *gma_connector = to_gma_connector(connector); + drm_connector_cleanup(connector); - kfree(connector); + kfree(gma_connector); } static bool psb_intel_sdvo_detect_hdmi_audio(struct drm_connector *connector) -- 2.35.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/7] drm/gma500: Declare a few functions static 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson ` (3 preceding siblings ...) 2022-03-22 13:17 ` [PATCH 5/7] drm/gma500: Free the correct connector allocation Patrik Jakobsson @ 2022-03-22 13:17 ` Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions Patrik Jakobsson 2022-03-22 19:30 ` [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Thomas Zimmermann 6 siblings, 0 replies; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 13:17 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, sam, tzimmermann These functions are not used outside of their file scope so can be declared as static. Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> --- drivers/gpu/drm/gma500/gma_display.c | 15 +++++++-------- drivers/gpu/drm/gma500/psb_drv.c | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c index e8157464d9eb..369bc1f751cb 100644 --- a/drivers/gpu/drm/gma500/gma_display.c +++ b/drivers/gpu/drm/gma500/gma_display.c @@ -176,9 +176,9 @@ void gma_crtc_load_lut(struct drm_crtc *crtc) } } -int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, - u32 size, - struct drm_modeset_acquire_ctx *ctx) +static int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, + u16 *blue, u32 size, + struct drm_modeset_acquire_ctx *ctx) { gma_crtc_load_lut(crtc); @@ -323,10 +323,9 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode) REG_WRITE(DSPARB, 0x3F3E); } -int gma_crtc_cursor_set(struct drm_crtc *crtc, - struct drm_file *file_priv, - uint32_t handle, - uint32_t width, uint32_t height) +static int gma_crtc_cursor_set(struct drm_crtc *crtc, + struct drm_file *file_priv, uint32_t handle, + uint32_t width, uint32_t height) { struct drm_device *dev = crtc->dev; struct drm_psb_private *dev_priv = to_drm_psb_private(dev); @@ -439,7 +438,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc, return ret; } -int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) +static int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) { struct drm_device *dev = crtc->dev; struct gma_crtc *gma_crtc = to_gma_crtc(crtc); diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index bb0e3288e35b..2aff54d505e2 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -100,7 +100,7 @@ static const struct drm_ioctl_desc psb_ioctls[] = { * * Soft reset the graphics engine and then reload the necessary registers. */ -void psb_spank(struct drm_psb_private *dev_priv) +static void psb_spank(struct drm_psb_private *dev_priv) { PSB_WSGX32(_PSB_CS_RESET_BIF_RESET | _PSB_CS_RESET_DPM_RESET | _PSB_CS_RESET_TA_RESET | _PSB_CS_RESET_USE_RESET | -- 2.35.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson ` (4 preceding siblings ...) 2022-03-22 13:17 ` [PATCH 6/7] drm/gma500: Declare a few functions static Patrik Jakobsson @ 2022-03-22 13:17 ` Patrik Jakobsson 2022-03-22 19:36 ` Thomas Zimmermann 2022-03-22 19:30 ` [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Thomas Zimmermann 6 siblings, 1 reply; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 13:17 UTC (permalink / raw) To: dri-devel; +Cc: daniel.vetter, sam, tzimmermann These functions operate on a crtc and should be prefixed properly. Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> --- drivers/gpu/drm/gma500/gma_display.c | 6 +++--- drivers/gpu/drm/gma500/psb_irq.c | 6 +++--- drivers/gpu/drm/gma500/psb_irq.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c index 369bc1f751cb..34ec3fca09ba 100644 --- a/drivers/gpu/drm/gma500/gma_display.c +++ b/drivers/gpu/drm/gma500/gma_display.c @@ -575,9 +575,9 @@ const struct drm_crtc_funcs gma_crtc_funcs = { .set_config = gma_crtc_set_config, .destroy = gma_crtc_destroy, .page_flip = gma_crtc_page_flip, - .enable_vblank = gma_enable_vblank, - .disable_vblank = gma_disable_vblank, - .get_vblank_counter = gma_get_vblank_counter, + .enable_vblank = gma_crtc_enable_vblank, + .disable_vblank = gma_crtc_disable_vblank, + .get_vblank_counter = gma_crtc_get_vblank_counter, }; /* diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c index 2e8ded532930..e6e6d61bbeab 100644 --- a/drivers/gpu/drm/gma500/psb_irq.c +++ b/drivers/gpu/drm/gma500/psb_irq.c @@ -371,7 +371,7 @@ void gma_irq_uninstall(struct drm_device *dev) free_irq(pdev->irq, dev); } -int gma_enable_vblank(struct drm_crtc *crtc) +int gma_crtc_enable_vblank(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; unsigned int pipe = crtc->index; @@ -404,7 +404,7 @@ int gma_enable_vblank(struct drm_crtc *crtc) return 0; } -void gma_disable_vblank(struct drm_crtc *crtc) +void gma_crtc_disable_vblank(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; unsigned int pipe = crtc->index; @@ -428,7 +428,7 @@ void gma_disable_vblank(struct drm_crtc *crtc) /* Called from drm generic code, passed a 'crtc', which * we use as a pipe index */ -u32 gma_get_vblank_counter(struct drm_crtc *crtc) +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; unsigned int pipe = crtc->index; diff --git a/drivers/gpu/drm/gma500/psb_irq.h b/drivers/gpu/drm/gma500/psb_irq.h index c22878914f5b..b51e395194ff 100644 --- a/drivers/gpu/drm/gma500/psb_irq.h +++ b/drivers/gpu/drm/gma500/psb_irq.h @@ -20,9 +20,9 @@ void gma_irq_postinstall(struct drm_device *dev); int gma_irq_install(struct drm_device *dev, unsigned int irq); void gma_irq_uninstall(struct drm_device *dev); -int gma_enable_vblank(struct drm_crtc *crtc); -void gma_disable_vblank(struct drm_crtc *crtc); -u32 gma_get_vblank_counter(struct drm_crtc *crtc); +int gma_crtc_enable_vblank(struct drm_crtc *crtc); +void gma_crtc_disable_vblank(struct drm_crtc *crtc); +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc); void gma_enable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask); void gma_disable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask); -- 2.35.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions 2022-03-22 13:17 ` [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions Patrik Jakobsson @ 2022-03-22 19:36 ` Thomas Zimmermann 2022-03-22 22:46 ` Patrik Jakobsson 0 siblings, 1 reply; 15+ messages in thread From: Thomas Zimmermann @ 2022-03-22 19:36 UTC (permalink / raw) To: Patrik Jakobsson, dri-devel; +Cc: daniel.vetter, sam [-- Attachment #1.1: Type: text/plain, Size: 3651 bytes --] Hi Am 22.03.22 um 14:17 schrieb Patrik Jakobsson: > These functions operate on a crtc and should be prefixed properly. > > Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> For the whole patchset: Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> If you like, please consider the few comments I had. Best regards Thomas > --- > drivers/gpu/drm/gma500/gma_display.c | 6 +++--- > drivers/gpu/drm/gma500/psb_irq.c | 6 +++--- > drivers/gpu/drm/gma500/psb_irq.h | 6 +++--- > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c > index 369bc1f751cb..34ec3fca09ba 100644 > --- a/drivers/gpu/drm/gma500/gma_display.c > +++ b/drivers/gpu/drm/gma500/gma_display.c > @@ -575,9 +575,9 @@ const struct drm_crtc_funcs gma_crtc_funcs = { > .set_config = gma_crtc_set_config, > .destroy = gma_crtc_destroy, > .page_flip = gma_crtc_page_flip, > - .enable_vblank = gma_enable_vblank, > - .disable_vblank = gma_disable_vblank, > - .get_vblank_counter = gma_get_vblank_counter, > + .enable_vblank = gma_crtc_enable_vblank, > + .disable_vblank = gma_crtc_disable_vblank, > + .get_vblank_counter = gma_crtc_get_vblank_counter, > }; > > /* > diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c > index 2e8ded532930..e6e6d61bbeab 100644 > --- a/drivers/gpu/drm/gma500/psb_irq.c > +++ b/drivers/gpu/drm/gma500/psb_irq.c > @@ -371,7 +371,7 @@ void gma_irq_uninstall(struct drm_device *dev) > free_irq(pdev->irq, dev); > } > > -int gma_enable_vblank(struct drm_crtc *crtc) > +int gma_crtc_enable_vblank(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > unsigned int pipe = crtc->index; > @@ -404,7 +404,7 @@ int gma_enable_vblank(struct drm_crtc *crtc) > return 0; > } > > -void gma_disable_vblank(struct drm_crtc *crtc) > +void gma_crtc_disable_vblank(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > unsigned int pipe = crtc->index; > @@ -428,7 +428,7 @@ void gma_disable_vblank(struct drm_crtc *crtc) > /* Called from drm generic code, passed a 'crtc', which > * we use as a pipe index > */ > -u32 gma_get_vblank_counter(struct drm_crtc *crtc) > +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > unsigned int pipe = crtc->index; > diff --git a/drivers/gpu/drm/gma500/psb_irq.h b/drivers/gpu/drm/gma500/psb_irq.h > index c22878914f5b..b51e395194ff 100644 > --- a/drivers/gpu/drm/gma500/psb_irq.h > +++ b/drivers/gpu/drm/gma500/psb_irq.h > @@ -20,9 +20,9 @@ void gma_irq_postinstall(struct drm_device *dev); > int gma_irq_install(struct drm_device *dev, unsigned int irq); > void gma_irq_uninstall(struct drm_device *dev); > > -int gma_enable_vblank(struct drm_crtc *crtc); > -void gma_disable_vblank(struct drm_crtc *crtc); > -u32 gma_get_vblank_counter(struct drm_crtc *crtc); > +int gma_crtc_enable_vblank(struct drm_crtc *crtc); > +void gma_crtc_disable_vblank(struct drm_crtc *crtc); > +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc); > void gma_enable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask); > void gma_disable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask); > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions 2022-03-22 19:36 ` Thomas Zimmermann @ 2022-03-22 22:46 ` Patrik Jakobsson 0 siblings, 0 replies; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 22:46 UTC (permalink / raw) To: Thomas Zimmermann; +Cc: Daniel Vetter, Sam Ravnborg, dri-devel On Tue, Mar 22, 2022 at 8:36 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > Am 22.03.22 um 14:17 schrieb Patrik Jakobsson: > > These functions operate on a crtc and should be prefixed properly. > > > > Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> > > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > > For the whole patchset: > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > > If you like, please consider the few comments I had. Thanks for the review. > > Best regards > Thomas > > > > --- > > drivers/gpu/drm/gma500/gma_display.c | 6 +++--- > > drivers/gpu/drm/gma500/psb_irq.c | 6 +++--- > > drivers/gpu/drm/gma500/psb_irq.h | 6 +++--- > > 3 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c > > index 369bc1f751cb..34ec3fca09ba 100644 > > --- a/drivers/gpu/drm/gma500/gma_display.c > > +++ b/drivers/gpu/drm/gma500/gma_display.c > > @@ -575,9 +575,9 @@ const struct drm_crtc_funcs gma_crtc_funcs = { > > .set_config = gma_crtc_set_config, > > .destroy = gma_crtc_destroy, > > .page_flip = gma_crtc_page_flip, > > - .enable_vblank = gma_enable_vblank, > > - .disable_vblank = gma_disable_vblank, > > - .get_vblank_counter = gma_get_vblank_counter, > > + .enable_vblank = gma_crtc_enable_vblank, > > + .disable_vblank = gma_crtc_disable_vblank, > > + .get_vblank_counter = gma_crtc_get_vblank_counter, > > }; > > > > /* > > diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c > > index 2e8ded532930..e6e6d61bbeab 100644 > > --- a/drivers/gpu/drm/gma500/psb_irq.c > > +++ b/drivers/gpu/drm/gma500/psb_irq.c > > @@ -371,7 +371,7 @@ void gma_irq_uninstall(struct drm_device *dev) > > free_irq(pdev->irq, dev); > > } > > > > -int gma_enable_vblank(struct drm_crtc *crtc) > > +int gma_crtc_enable_vblank(struct drm_crtc *crtc) > > { > > struct drm_device *dev = crtc->dev; > > unsigned int pipe = crtc->index; > > @@ -404,7 +404,7 @@ int gma_enable_vblank(struct drm_crtc *crtc) > > return 0; > > } > > > > -void gma_disable_vblank(struct drm_crtc *crtc) > > +void gma_crtc_disable_vblank(struct drm_crtc *crtc) > > { > > struct drm_device *dev = crtc->dev; > > unsigned int pipe = crtc->index; > > @@ -428,7 +428,7 @@ void gma_disable_vblank(struct drm_crtc *crtc) > > /* Called from drm generic code, passed a 'crtc', which > > * we use as a pipe index > > */ > > -u32 gma_get_vblank_counter(struct drm_crtc *crtc) > > +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc) > > { > > struct drm_device *dev = crtc->dev; > > unsigned int pipe = crtc->index; > > diff --git a/drivers/gpu/drm/gma500/psb_irq.h b/drivers/gpu/drm/gma500/psb_irq.h > > index c22878914f5b..b51e395194ff 100644 > > --- a/drivers/gpu/drm/gma500/psb_irq.h > > +++ b/drivers/gpu/drm/gma500/psb_irq.h > > @@ -20,9 +20,9 @@ void gma_irq_postinstall(struct drm_device *dev); > > int gma_irq_install(struct drm_device *dev, unsigned int irq); > > void gma_irq_uninstall(struct drm_device *dev); > > > > -int gma_enable_vblank(struct drm_crtc *crtc); > > -void gma_disable_vblank(struct drm_crtc *crtc); > > -u32 gma_get_vblank_counter(struct drm_crtc *crtc); > > +int gma_crtc_enable_vblank(struct drm_crtc *crtc); > > +void gma_crtc_disable_vblank(struct drm_crtc *crtc); > > +u32 gma_crtc_get_vblank_counter(struct drm_crtc *crtc); > > void gma_enable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask); > > void gma_disable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 mask); > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson ` (5 preceding siblings ...) 2022-03-22 13:17 ` [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions Patrik Jakobsson @ 2022-03-22 19:30 ` Thomas Zimmermann 2022-03-22 19:32 ` Thomas Zimmermann 6 siblings, 1 reply; 15+ messages in thread From: Thomas Zimmermann @ 2022-03-22 19:30 UTC (permalink / raw) To: Patrik Jakobsson, dri-devel; +Cc: daniel.vetter, sam [-- Attachment #1.1: Type: text/plain, Size: 1317 bytes --] Hi Patrik Am 22.03.22 um 14:17 schrieb Patrik Jakobsson: > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > --- > drivers/gpu/drm/gma500/framebuffer.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c > index 45df9de22007..2b99c996fdc2 100644 > --- a/drivers/gpu/drm/gma500/framebuffer.c > +++ b/drivers/gpu/drm/gma500/framebuffer.c > @@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev) > struct pci_dev *pdev = to_pci_dev(dev->dev); > int i; > > - drm_mode_config_init(dev); > + if (drmm_mode_config_init(dev)) > + return; This will fail without any notice. I suggest to return an error here or at let psb_driver_load() fail. Best regards Thomas > > dev->mode_config.min_width = 0; > dev->mode_config.min_height = 0; > @@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev) > if (dev_priv->modeset) { > drm_kms_helper_poll_fini(dev); > psb_fbdev_fini(dev); > - drm_mode_config_cleanup(dev); > } > } -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() 2022-03-22 19:30 ` [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Thomas Zimmermann @ 2022-03-22 19:32 ` Thomas Zimmermann 2022-03-22 22:42 ` Patrik Jakobsson 0 siblings, 1 reply; 15+ messages in thread From: Thomas Zimmermann @ 2022-03-22 19:32 UTC (permalink / raw) To: Patrik Jakobsson, dri-devel; +Cc: daniel.vetter, sam [-- Attachment #1.1: Type: text/plain, Size: 1567 bytes --] Am 22.03.22 um 20:30 schrieb Thomas Zimmermann: > Hi Patrik > > Am 22.03.22 um 14:17 schrieb Patrik Jakobsson: >> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >> --- >> drivers/gpu/drm/gma500/framebuffer.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/gma500/framebuffer.c >> b/drivers/gpu/drm/gma500/framebuffer.c >> index 45df9de22007..2b99c996fdc2 100644 >> --- a/drivers/gpu/drm/gma500/framebuffer.c >> +++ b/drivers/gpu/drm/gma500/framebuffer.c >> @@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev) >> struct pci_dev *pdev = to_pci_dev(dev->dev); >> int i; >> - drm_mode_config_init(dev); >> + if (drmm_mode_config_init(dev)) >> + return; > > This will fail without any notice. I suggest to return an error here or > at let psb_driver_load() fail. 'and let psb_driver_load() fail' > > Best regards > Thomas > >> dev->mode_config.min_width = 0; >> dev->mode_config.min_height = 0; >> @@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev) >> if (dev_priv->modeset) { >> drm_kms_helper_poll_fini(dev); >> psb_fbdev_fini(dev); >> - drm_mode_config_cleanup(dev); >> } >> } > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() 2022-03-22 19:32 ` Thomas Zimmermann @ 2022-03-22 22:42 ` Patrik Jakobsson 0 siblings, 0 replies; 15+ messages in thread From: Patrik Jakobsson @ 2022-03-22 22:42 UTC (permalink / raw) To: Thomas Zimmermann; +Cc: Daniel Vetter, Sam Ravnborg, dri-devel On Tue, Mar 22, 2022 at 8:32 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > > > Am 22.03.22 um 20:30 schrieb Thomas Zimmermann: > > Hi Patrik > > > > Am 22.03.22 um 14:17 schrieb Patrik Jakobsson: > >> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > >> --- > >> drivers/gpu/drm/gma500/framebuffer.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/gma500/framebuffer.c > >> b/drivers/gpu/drm/gma500/framebuffer.c > >> index 45df9de22007..2b99c996fdc2 100644 > >> --- a/drivers/gpu/drm/gma500/framebuffer.c > >> +++ b/drivers/gpu/drm/gma500/framebuffer.c > >> @@ -514,7 +514,8 @@ void psb_modeset_init(struct drm_device *dev) > >> struct pci_dev *pdev = to_pci_dev(dev->dev); > >> int i; > >> - drm_mode_config_init(dev); > >> + if (drmm_mode_config_init(dev)) > >> + return; > > > > This will fail without any notice. I suggest to return an error here or > > at let psb_driver_load() fail. > > 'and let psb_driver_load() fail' Hi Thomas, I did consider it but there are more places where psb_driver_load() can fail so I think it deserves its own patch. I'll send a follow-up. -Patrik > > > > > Best regards > > Thomas > > > >> dev->mode_config.min_width = 0; > >> dev->mode_config.min_height = 0; > >> @@ -546,6 +547,5 @@ void psb_modeset_cleanup(struct drm_device *dev) > >> if (dev_priv->modeset) { > >> drm_kms_helper_poll_fini(dev); > >> psb_fbdev_fini(dev); > >> - drm_mode_config_cleanup(dev); > >> } > >> } > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-03-22 22:46 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-22 13:17 [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 2/7] drm/gma500: Uninstall interrupts on driver removal Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 3/7] drm/gma500: Make use of the drm connector iterator Patrik Jakobsson 2022-03-22 14:39 ` Daniel Vetter 2022-03-22 15:46 ` Patrik Jakobsson 2022-03-22 19:34 ` Thomas Zimmermann 2022-03-22 13:17 ` [PATCH 4/7] drm/gma500: gma500 don't register non-hotpluggable connectors Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 5/7] drm/gma500: Free the correct connector allocation Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 6/7] drm/gma500: Declare a few functions static Patrik Jakobsson 2022-03-22 13:17 ` [PATCH 7/7] drm/gma500: Add crtc prefix to vblank functions Patrik Jakobsson 2022-03-22 19:36 ` Thomas Zimmermann 2022-03-22 22:46 ` Patrik Jakobsson 2022-03-22 19:30 ` [PATCH 1/7] drm/gma500: Use managed drmm_mode_config_init() Thomas Zimmermann 2022-03-22 19:32 ` Thomas Zimmermann 2022-03-22 22:42 ` Patrik Jakobsson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.