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

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

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

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);       }   }