Re: [PATCH 2/6] drm/nouveau: Pass mode-dependent AVI and Vendor HDMI InfoFrames to NVKM

[Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>]

On Tue, Jan 17, 2017 at 5:42 PM, Alastair Bridgewater
<alastair.bridgewater@gmail.com> wrote:
> Now that we have mechanism by which to pass mode-dependent HDMI
> InfoFrames to the low-level hardware driver, it is incumbent upon
> us to do so.
>
> Experimentation on a gt215 device suggests that the Audio InfoFrame
> is not required here, possibly being provided by the HDA device
> when necessary (because where else would it come from?).

Presumably it's necessary on G84, which doesn't have the HDA device?
Looks like there's no helper for computing such a thing in drm_edid.
It's a pretty fixed setup on G84... you're supposed to hook the audio
from your sound card into an internal S/PDIF connector, so just
leaving the default audio infoframe on in there might be enough.

>
> Signed-off-by: Alastair Bridgewater <alastair.bridgewater@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nv50_display.c | 49 +++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 2c2c645..d52d0b8 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -23,6 +23,7 @@
>   */
>
>  #include <linux/dma-mapping.h>
> +#include <linux/hdmi.h>
>
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic.h>
> @@ -31,6 +32,7 @@
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_plane_helper.h>
> +#include <drm/drm_edid.h>
>
>  #include <nvif/class.h>
>  #include <nvif/cl0002.h>
> @@ -2772,6 +2774,28 @@ nv50_hdmi_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
>         nvif_mthd(disp->disp, 0, &args, sizeof(args));
>  }
>
> +static ssize_t
> +nv50_hdmi_pack_infoframe(struct nv50_disp_sor_hdmi_pwr_v0_infoframe *frame_out,
> +                        union hdmi_infoframe *frame_in)
> +{
> +       uint8_t buffer[17]; /* The header plus two "subpacks" */
> +       ssize_t len;
> +
> +       len = hdmi_infoframe_pack(frame_in, buffer, sizeof(buffer));
> +
> +       frame_out->header = buffer[0] | (buffer[1] << 8) | (buffer[2] << 16);
> +       frame_out->subpack0_low = buffer[3] | (buffer[4] << 8) |
> +               (buffer[5] << 16) | (buffer[6] << 24);
> +       frame_out->subpack0_high = buffer[7] | (buffer[8] << 8) |
> +               (buffer[9] << 16);
> +       frame_out->subpack1_low = buffer[10] | (buffer[11] << 8) |
> +               (buffer[12] << 16) | (buffer[13] << 24);
> +       frame_out->subpack1_high = buffer[14] | (buffer[15] << 8) |
> +               (buffer[16] << 16);
> +
> +       return len;
> +}
> +
>  static void
>  nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
>  {
> @@ -2781,6 +2805,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
>         struct {
>                 struct nv50_disp_mthd_v1 base;
>                 struct nv50_disp_sor_hdmi_pwr_v0 pwr;
> +               struct nv50_disp_sor_hdmi_pwr_v0_infoframe iframe[3];
>         } args = {
>                 .base.version = 1,
>                 .base.method = NV50_DISP_MTHD_V1_SOR_HDMI_PWR,
> @@ -2792,17 +2817,39 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
>         };
>         struct nouveau_connector *nv_connector;
>         u32 max_ac_packet;
> +       union hdmi_infoframe avi_frame;
> +       union hdmi_infoframe vendor_frame;
> +       int ret;
> +       int size;
> +       int frame = 0;
>
>         nv_connector = nouveau_encoder_connector_get(nv_encoder);
>         if (!drm_detect_hdmi_monitor(nv_connector->edid))
>                 return;
>
> +       /* Audio InfoFrame apparently not required (supplied by HDA device?) */
> +
> +       ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode);
> +       if (ret >= 0) {

if (!ret) or if (ret == 0) is more idiomatic.

> +               /* We have an AVI InfoFrame, populate it to the display */
> +               args.pwr.flags |= NV50_DISP_MTHD_V1_SOR_HDMI_PWR_FLAG_AVI_INFOFRAME;
> +               nv50_hdmi_pack_infoframe(&args.iframe[frame++], &avi_frame);
> +       }
> +
> +       ret = drm_hdmi_vendor_infoframe_from_display_mode(&vendor_frame.vendor.hdmi, mode);
> +       if (ret >= 0) {
> +               /* We have a Vendor InfoFrame, populate it to the display */
> +               args.pwr.flags |= NV50_DISP_MTHD_V1_SOR_HDMI_PWR_FLAG_VENDOR_INFOFRAME;
> +               nv50_hdmi_pack_infoframe(&args.iframe[frame++], &vendor_frame);
> +       }
> +
>         max_ac_packet  = mode->htotal - mode->hdisplay;
>         max_ac_packet -= args.pwr.rekey;
>         max_ac_packet -= 18; /* constant from tegra */
>         args.pwr.max_ac_packet = max_ac_packet / 32;
>
> -       nvif_mthd(disp->disp, 0, &args, sizeof(args));
> +       size = sizeof(args.base) + sizeof(args.pwr) + frame * sizeof(args.iframe[0]);
> +       nvif_mthd(disp->disp, 0, &args, size);
>         nv50_audio_enable(encoder, mode);
>  }
>
> --
> 2.10.2
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau