Re: [PATCH] drm/dp_mst: Remove drm_dp_mst_has_audio()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 3, 2020 at 6:23 PM Lyude Paul <lyude@xxxxxxxxxx> wrote:
>
> Drive-by fix I noticed the other day - drm_dp_mst_has_audio() only ever
> made sense back when we still had to validate ports before accessing
> them in order to (attempt to) avoid NULL dereferences. Since we have
> proper reference counting that guarantees we always can safely access
> the MST port, there's no use in keeping this function around as all it
> does is validate the port pointer before checking the audio status.
>
> Note - drm_dp_mst_port->has_audio is technically protected by
> drm_device->mode_config.connection_mutex, since it's only ever updated
> from drm_dp_mst_get_edid(). Additionally, we change the declaration for
> port in struct intel_connector to be properly typed, so we can directly
> access it.
>

This is kind of burying the lede. I'd almost prefer a 2 patch series:

drm/i915: Allow connectors to directly access drm_dp_mst_port
drm/dp_mst: Remove unused drm_dp_mst_port_has_audio()

That way if anyone objects to the idea of accessing mst_port directly
from i915 driver, it's more obvious from the patch subject.

Nitpicks aside, the code looks good to me, it's a nice cleanup!

Reviewed-by: Sean Paul <sean@xxxxxxxxxx>

> Cc: "Lee, Shawn C" <shawn.c.lee@xxxxxxxxx>
> Cc: Sean Paul <sean@xxxxxxxxxx>
> Signed-off-by: Lyude Paul <lyude@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c         | 21 -------------------
>  .../drm/i915/display/intel_display_debugfs.c  | 10 ++-------
>  .../drm/i915/display/intel_display_types.h    |  2 +-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  3 +--
>  include/drm/drm_dp_mst_helper.h               |  2 --
>  5 files changed, 4 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1ff49547b2e8..129126091e90 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4063,27 +4063,6 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_detect_port);
>
> -/**
> - * drm_dp_mst_port_has_audio() - Check whether port has audio capability or not
> - * @mgr: manager for this port
> - * @port: unverified pointer to a port.
> - *
> - * This returns whether the port supports audio or not.
> - */
> -bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
> -                                       struct drm_dp_mst_port *port)
> -{
> -       bool ret = false;
> -
> -       port = drm_dp_mst_topology_get_port_validated(mgr, port);
> -       if (!port)
> -               return ret;
> -       ret = port->has_audio;
> -       drm_dp_mst_topology_put_port(port);
> -       return ret;
> -}
> -EXPORT_SYMBOL(drm_dp_mst_port_has_audio);
> -
>  /**
>   * drm_dp_mst_get_edid() - get EDID for an MST port
>   * @connector: toplevel connector to get EDID for
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 424f4e52f783..9f736420d83f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -631,15 +631,9 @@ static void intel_dp_info(struct seq_file *m,
>  }
>
>  static void intel_dp_mst_info(struct seq_file *m,
> -                         struct intel_connector *intel_connector)
> +                             struct intel_connector *intel_connector)
>  {
> -       struct intel_encoder *intel_encoder = intel_attached_encoder(intel_connector);
> -       struct intel_dp_mst_encoder *intel_mst =
> -               enc_to_mst(intel_encoder);
> -       struct intel_digital_port *intel_dig_port = intel_mst->primary;
> -       struct intel_dp *intel_dp = &intel_dig_port->dp;
> -       bool has_audio = drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
> -                                       intel_connector->port);
> +       bool has_audio = intel_connector->port->has_audio;
>
>         seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 2bedd626c686..1de7bef0a49b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -436,7 +436,7 @@ struct intel_connector {
>            state of connector->polled in case hotplug storm detection changes it */
>         u8 polled;
>
> -       void *port; /* store this opaque as its illegal to dereference it */
> +       struct drm_dp_mst_port *port;
>
>         struct intel_dp *mst_port;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 61605eb8c2af..c35efc9e628d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -114,8 +114,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>
>         if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
>                 pipe_config->has_audio =
> -                       drm_dp_mst_port_has_audio(&intel_dp->mst_mgr,
> -                                                 connector->port);
> +                       connector->port->has_audio;
>         else
>                 pipe_config->has_audio =
>                         intel_conn_state->force_audio == HDMI_AUDIO_ON;
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 7af51c947b81..2d7c26592c05 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -732,8 +732,6 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
>                        struct drm_dp_mst_topology_mgr *mgr,
>                        struct drm_dp_mst_port *port);
>
> -bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
> -                                       struct drm_dp_mst_port *port);
>  struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
>
>
> --
> 2.25.1
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux