[PATCH 3/7] drm/gma500: Make use of the drm connector iterator
Thomas Zimmermann
tzimmermann at suse.de
Tue Mar 22 19:34:28 UTC 2022
Hi
Am 22.03.22 um 16:46 schrieb Patrik Jakobsson:
> On Tue, Mar 22, 2022 at 3:39 PM Daniel Vetter <daniel at 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 at 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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220322/57b22f86/attachment-0001.sig>
More information about the dri-devel
mailing list