Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator

22 Mar 2022

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

    

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

Re: [PATCH 3/7] drm/gma500: Make use of the drm connector iterator