Re: 答复: [PATCH 4/4] drm/amdgpu: Add audio component support

[jimqu <jimqu@amd.com>]

On 2018年07月25日 13:46, Takashi Iwai wrote:
> On Wed, 25 Jul 2018 07:38:37 +0200,
> Qu, Jim wrote:
>> Jim: Just like Alex said, we want driver can get eld info when hotplug in new device. amdgpu driver is a bit difference from radeon driver, it is not a suitable place to call notify() function in *_audio_enable() , since they are not in the hotplug process context like them in radeon driver, but the mode setting.
>>
>> IMO, the right place to call notify() function is also in the amdgpu_connector_xxxx_detect() in amdgpu_connector.c.
> Hm, but by the modesetting, it actually enables / disables the audio
> as well, no?  If so, the notifier is exactly for that purpose.  The
> audio driver needs to know not only about the physical connection but
> whether the audio can be actually driven.
>
> That is, even if the monitor is connected, the audio won't come out if
> the mode is off.  That is equivalent with the unplugged state for the
> audio driver.
>
> The i915 driver deals with the notifier just like the above, so the
> behavior is intentional.
>
>
> thanks,
>
> Takashi

I am afraid if device hotplug out, how is audio state if it follow up 
eld info? Since the modesetting is never performed for the display which 
is plugged out, so there is no notify() call on it.

Thanks
JimQu
>> With None-DC driver, the detect() of connectors will to be preformed in drm_helper_hpd_irq_event() when there is device hotplug. However, amdgpu_connector_xxxx_detect() maybe need to be modified to fit your change.
>>
>> Thanks
>> JimQu
>> ************************************************************************************************************
>> ---
>>   drivers/gpu/drm/Kconfig                   |  1 +
>>   drivers/gpu/drm/amd/amdgpu/Makefile       |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c | 97 +++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  3 +
>>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  6 ++
>>   drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  6 ++
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  6 ++
>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  6 ++
>>   9 files changed, 130 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 2c7112ddfed4..fbe7216c5c56 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -193,6 +193,7 @@ config DRM_AMDGPU
>>          select BACKLIGHT_LCD_SUPPORT
>>          select INTERVAL_TREE
>>          select CHASH
>> +       select SND_HDA_COMPONENT if SND_HDA_CORE
>>          help
>>            Choose this option if you have a recent AMD Radeon graphics card.
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
>> index bfd332c95b61..9c26facddb17 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>> @@ -52,7 +52,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>>          amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
>>          amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_atomfirmware.o \
>>          amdgpu_queue_mgr.o amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o \
>> -       amdgpu_ids.o
>> +       amdgpu_ids.o amdgpu_audio.o
>>
>>   # add asic specific block
>>   amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index a59c07590cee..203d2584c989 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1957,5 +1957,9 @@ int amdgpu_dm_display_resume(struct amdgpu_device *adev );
>>   static inline int amdgpu_dm_display_resume(struct amdgpu_device *adev) { return 0; }
>>   #endif
>>
>> +int amdgpu_audio_component_init(struct amdgpu_device *adev);
>> +void amdgpu_audio_component_fini(struct amdgpu_device *adev);
>> +void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin);
>> +
>>   #include "amdgpu_object.h"
>>   #endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
>> new file mode 100644
>> index 000000000000..39256e2f84b3
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
>> @@ -0,0 +1,97 @@
>> +// SPDX-License-Identifier: MIT
>> +
>> +#include <linux/component.h>
>> +#include "amdgpu.h"
>> +
>> +static int amdgpu_audio_component_get_eld(struct device *kdev, int port,
>> +                                         int pipe, bool *enabled,
>> +                                         unsigned char *buf, int max_bytes)
>> +{
>> +       struct drm_device *dev = dev_get_drvdata(kdev);
>> +       struct drm_encoder *encoder;
>> +       struct amdgpu_encoder *amdgpu_encoder;
>> +       struct amdgpu_encoder_atom_dig *dig;
>> +       struct drm_connector *connector;
>> +       int ret = 0;
>> +
>> +       *enabled = 0;
>> +       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
>> +               amdgpu_encoder = to_amdgpu_encoder(encoder);
>> +               dig = amdgpu_encoder->enc_priv;
>> +               if (!dig || !dig->afmt || !dig->afmt->enabled)
>> +                       continue;
>> +               if (!dig->afmt->pin || dig->afmt->pin->id != port)
>> +                       continue;
>> +               connector = amdgpu_get_connector_for_encoder(encoder);
>> +               *enabled = !!connector;
>> +               if (connector) {
>> +                       ret = drm_eld_size(connector->eld);
>> +                       memcpy(buf, connector->eld, min(max_bytes, ret));
>> +                       break;
>> +               }
>> +       }
>> +
>> +       return ret;
>> +}
>> +
>> +static const struct drm_audio_component_ops amdgpu_audio_component_ops = {
>> +       .get_eld = amdgpu_audio_component_get_eld,
>> +};
>> +
>> +static int amdgpu_audio_component_bind(struct device *kdev,
>> +                                      struct device *hda_kdev, void *data)
>> +{
>> +       struct drm_device *dev = dev_get_drvdata(kdev);
>> +       struct amdgpu_device *adev = dev->dev_private;
>> +       struct drm_audio_component *acomp = data;
>> +
>> +       acomp->ops = &amdgpu_audio_component_ops;
>> +       acomp->dev = kdev;
>> +       adev->mode_info.audio.component = acomp;
>> +       return 0;
>> +}
>> +
>> +static void amdgpu_audio_component_unbind(struct device *kdev,
>> +                                         struct device *hda_kdev, void *data)
>> +{
>> +       struct drm_device *dev = dev_get_drvdata(kdev);
>> +       struct amdgpu_device *adev = dev->dev_private;
>> +       struct drm_audio_component *acomp = data;
>> +
>> +       acomp->ops = NULL;
>> +       acomp->dev = NULL;
>> +       adev->mode_info.audio.component = NULL;
>> +}
>> +
>> +static const struct component_ops amdgpu_audio_component_bind_ops = {
>> +       .bind   = amdgpu_audio_component_bind,
>> +       .unbind = amdgpu_audio_component_unbind,
>> +};
>> +
>> +int amdgpu_audio_component_init(struct amdgpu_device *adev)
>> +{
>> +       int err;
>> +
>> +       err = component_add(adev->dev, &amdgpu_audio_component_bind_ops);
>> +       if (err < 0)
>> +               return err;
>> +       adev->mode_info.audio.component_registered = true;
>> +       return 0;
>> +}
>> +
>> +void amdgpu_audio_component_fini(struct amdgpu_device *adev)
>> +{
>> +       if (adev->mode_info.audio.component_registered) {
>> +               component_del(adev->dev, &amdgpu_audio_component_bind_ops);
>> +               adev->mode_info.audio.component_registered = false;
>> +       }
>> +}
>> +
>> +void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin)
>> +{
>> +       struct drm_audio_component *acomp = adev->mode_info.audio.component;
>> +
>> +       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
>> +               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
>> +                                                pin, -1);
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> index b9e9e8b02fb7..7cb1aa97e522 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> @@ -39,6 +39,7 @@
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_plane_helper.h>
>>   #include <drm/drm_fb_helper.h>
>> +#include <drm/drm_audio_component.h>
>>   #include <linux/i2c.h>
>>   #include <linux/i2c-algo-bit.h>
>>   #include <linux/hrtimer.h>
>> @@ -260,6 +261,8 @@ struct amdgpu_audio {
>>          bool enabled;
>>          struct amdgpu_audio_pin pin[AMDGPU_MAX_AFMT_BLOCKS];
>>          int num_pins;
>> +       bool component_registered;
>> +       struct drm_audio_component *component;
>>   };
>>
>>   struct amdgpu_display_funcs {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> index ada241bfeee9..c8471a59930f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> @@ -1392,6 +1392,8 @@ static void dce_v10_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                             enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[] =
>> @@ -1430,6 +1432,8 @@ static int dce_v10_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1443,6 +1447,8 @@ static void dce_v10_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> index a5b96eac3033..49edb74725a7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> @@ -1418,6 +1418,8 @@ static void dce_v11_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                             enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[] =
>> @@ -1472,6 +1474,8 @@ static int dce_v11_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1485,6 +1489,8 @@ static void dce_v11_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> index 394cc1e8fe20..2b7468eaacca 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> @@ -1297,6 +1297,8 @@ static void dce_v6_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                          enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[7] =
>> @@ -1343,6 +1345,8 @@ static int dce_v6_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1356,6 +1360,8 @@ static void dce_v6_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> index c9b9ab8f1b05..c2370ea1f64f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> @@ -1348,6 +1348,8 @@ static void dce_v8_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                  enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[7] =
>> @@ -1395,6 +1397,8 @@ static int dce_v8_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1408,6 +1412,8 @@ static void dce_v8_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> --
>> 2.18.0
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel