| Message ID | 20221011170011.17198-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915: ELD precompute and readout | expand |
On Tue, 11 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > There was some discussion around ELD precompute, so I > decided to have a quick look at hooking that up. Unfortunately > the i915 audio code is a bit of a mess so ended up with > a patchbomb of cleanups. Sorry about that. > > The actually interesting stuff is at the end of the series. > The precumpute+readot+state checker is pretty self explanatory > stuff for the most part. > > But I think we need to decide what to do with the hardware > ELD buffer in general. It's totally busted atm on HSW > (and I'd expecpt BDW as well), but we had no idea since > we had no readout+state checker for it. > > So do we try to salvage it (I guess to mainly act as some > kind of "did we enable audio correctly?" canary) or do we > just stop programming it outright? And on which platforms > could we do that? Quickly eyeballed through it all, overall looks good, but didn't do detailed review. Ack. Cc: Vinod too. BR, Jani. > > Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com> > Cc: Takashi Iwai <tiwai@suse.de> > > Ville Syrjälä (22): > drm/i915/audio: s/dev_priv/i915/ > drm/i915/audio: Nuke leftover ROUNDING_FACTOR > drm/i915/audio: Remove CL/BLC audio stuff > drm/i915/audio: Exract struct ilk_audio_regs > drm/i915/audio: Use REG_BIT() & co. > drm/i915/audio: Unify register bit naming > drm/i915/audio: Protect singleton register with a lock > drm/i915/audio: Nuke intel_eld_uptodate() > drm/i915/audio: Read ELD buffer size from hardware > drm/i915/audio: Make sure we write the whole ELD buffer > drm/i915/audio: Use u32* for ELD > drm/i915/audio: Use intel_de_rmw() for most audio registers > drm/i915/audio: Split "ELD valid" vs. audio PD on hsw+ > drm/i915/audio: Do the vblank waits > drm/i915/audio: Precompute the ELD > drm/i915/audio: Hardware ELD readout > drm/i915/sdvo: Extract intel_sdvo_has_audio() > drm/i915/sdvo: Precompute the ELD > drm/i915/sdvo: Do ELD hardware readout > drm/i915/audio: Hook up ELD into the state checker > drm/i915/audio: Include ELD in the state dump > hax: drm/i915/audio: Make HSW hardware ELD buffer sort of work > > drivers/gpu/drm/i915/display/g4x_dp.c | 2 + > drivers/gpu/drm/i915/display/g4x_hdmi.c | 2 + > drivers/gpu/drm/i915/display/intel_audio.c | 792 ++++++++++-------- > drivers/gpu/drm/i915/display/intel_audio.h | 7 + > .../gpu/drm/i915/display/intel_audio_regs.h | 88 +- > .../drm/i915/display/intel_crtc_state_dump.c | 17 + > drivers/gpu/drm/i915/display/intel_ddi.c | 2 + > drivers/gpu/drm/i915/display/intel_display.c | 43 + > .../drm/i915/display/intel_display_types.h | 2 + > drivers/gpu/drm/i915/display/intel_dp.c | 4 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +- > drivers/gpu/drm/i915/display/intel_sdvo.c | 54 +- > 12 files changed, 608 insertions(+), 409 deletions(-)
> -----Original Message----- > From: Ville Syrjala <ville.syrjala@linux.intel.com> > Sent: Tuesday, October 11, 2022 10:30 PM > To: intel-gfx@lists.freedesktop.org > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Kai > Vehmanen <kai.vehmanen@linux.intel.com>; Takashi Iwai <tiwai@suse.de> > Subject: [PATCH 00/22] drm/i915: ELD precompute and readout > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > There was some discussion around ELD precompute, so I decided to have a > quick look at hooking that up. Unfortunately the i915 audio code is a bit of a > mess so ended up with a patchbomb of cleanups. Sorry about that. > > The actually interesting stuff is at the end of the series. > The precumpute+readot+state checker is pretty self explanatory stuff for the > most part. > > But I think we need to decide what to do with the hardware ELD buffer in > general. It's totally busted atm on HSW (and I'd expecpt BDW as well), but > we had no idea since we had no readout+state checker for it. > > So do we try to salvage it (I guess to mainly act as some kind of "did we > enable audio correctly?" canary) or do we just stop programming it outright? > And on which platforms could we do that? > Hello Ville, I have gone through the patches and they look good. However, there is one aspect that is still not clear for me(may be I have missed something!) The changes does not touch the callback i915_audio_component_get_eld() which is actually used by the audio driver to access the ELD. So we are still sending out an "non-precomputed" eld stored in the connector structure. Should we be passing eld data from crtc_state instead here? Regards, Chaitanya > Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com> > Cc: Takashi Iwai <tiwai@suse.de> > > Ville Syrjälä (22): > drm/i915/audio: s/dev_priv/i915/ > drm/i915/audio: Nuke leftover ROUNDING_FACTOR > drm/i915/audio: Remove CL/BLC audio stuff > drm/i915/audio: Exract struct ilk_audio_regs > drm/i915/audio: Use REG_BIT() & co. > drm/i915/audio: Unify register bit naming > drm/i915/audio: Protect singleton register with a lock > drm/i915/audio: Nuke intel_eld_uptodate() > drm/i915/audio: Read ELD buffer size from hardware > drm/i915/audio: Make sure we write the whole ELD buffer > drm/i915/audio: Use u32* for ELD > drm/i915/audio: Use intel_de_rmw() for most audio registers > drm/i915/audio: Split "ELD valid" vs. audio PD on hsw+ > drm/i915/audio: Do the vblank waits > drm/i915/audio: Precompute the ELD > drm/i915/audio: Hardware ELD readout > drm/i915/sdvo: Extract intel_sdvo_has_audio() > drm/i915/sdvo: Precompute the ELD > drm/i915/sdvo: Do ELD hardware readout > drm/i915/audio: Hook up ELD into the state checker > drm/i915/audio: Include ELD in the state dump > hax: drm/i915/audio: Make HSW hardware ELD buffer sort of work > > drivers/gpu/drm/i915/display/g4x_dp.c | 2 + > drivers/gpu/drm/i915/display/g4x_hdmi.c | 2 + > drivers/gpu/drm/i915/display/intel_audio.c | 792 ++++++++++-------- > drivers/gpu/drm/i915/display/intel_audio.h | 7 + > .../gpu/drm/i915/display/intel_audio_regs.h | 88 +- > .../drm/i915/display/intel_crtc_state_dump.c | 17 + > drivers/gpu/drm/i915/display/intel_ddi.c | 2 + > drivers/gpu/drm/i915/display/intel_display.c | 43 + > .../drm/i915/display/intel_display_types.h | 2 + > drivers/gpu/drm/i915/display/intel_dp.c | 4 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +- > drivers/gpu/drm/i915/display/intel_sdvo.c | 54 +- > 12 files changed, 608 insertions(+), 409 deletions(-) > > -- > 2.35.1
On Fri, 14 Oct 2022, "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com> wrote: >> -----Original Message----- >> From: Ville Syrjala <ville.syrjala@linux.intel.com> >> Sent: Tuesday, October 11, 2022 10:30 PM >> To: intel-gfx@lists.freedesktop.org >> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Kai >> Vehmanen <kai.vehmanen@linux.intel.com>; Takashi Iwai <tiwai@suse.de> >> Subject: [PATCH 00/22] drm/i915: ELD precompute and readout >> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> There was some discussion around ELD precompute, so I decided to have a >> quick look at hooking that up. Unfortunately the i915 audio code is a bit of a >> mess so ended up with a patchbomb of cleanups. Sorry about that. >> >> The actually interesting stuff is at the end of the series. >> The precumpute+readot+state checker is pretty self explanatory stuff for the >> most part. >> >> But I think we need to decide what to do with the hardware ELD buffer in >> general. It's totally busted atm on HSW (and I'd expecpt BDW as well), but >> we had no idea since we had no readout+state checker for it. >> >> So do we try to salvage it (I guess to mainly act as some kind of "did we >> enable audio correctly?" canary) or do we just stop programming it outright? >> And on which platforms could we do that? >> > > Hello Ville, > > I have gone through the patches and they look good. However, there is one aspect that is still not clear for me(may be I have missed something!) > The changes does not touch the callback i915_audio_component_get_eld() which is actually used by the audio driver to access the ELD. So we are still sending out an "non-precomputed" eld stored in the connector > structure. Should we be passing eld data from crtc_state instead here? Good catch, I missed that in my review. BR, Jani. > > Regards, > > Chaitanya > >> Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> >> Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com> >> Cc: Takashi Iwai <tiwai@suse.de> >> >> Ville Syrjälä (22): >> drm/i915/audio: s/dev_priv/i915/ >> drm/i915/audio: Nuke leftover ROUNDING_FACTOR >> drm/i915/audio: Remove CL/BLC audio stuff >> drm/i915/audio: Exract struct ilk_audio_regs >> drm/i915/audio: Use REG_BIT() & co. >> drm/i915/audio: Unify register bit naming >> drm/i915/audio: Protect singleton register with a lock >> drm/i915/audio: Nuke intel_eld_uptodate() >> drm/i915/audio: Read ELD buffer size from hardware >> drm/i915/audio: Make sure we write the whole ELD buffer >> drm/i915/audio: Use u32* for ELD >> drm/i915/audio: Use intel_de_rmw() for most audio registers >> drm/i915/audio: Split "ELD valid" vs. audio PD on hsw+ >> drm/i915/audio: Do the vblank waits >> drm/i915/audio: Precompute the ELD >> drm/i915/audio: Hardware ELD readout >> drm/i915/sdvo: Extract intel_sdvo_has_audio() >> drm/i915/sdvo: Precompute the ELD >> drm/i915/sdvo: Do ELD hardware readout >> drm/i915/audio: Hook up ELD into the state checker >> drm/i915/audio: Include ELD in the state dump >> hax: drm/i915/audio: Make HSW hardware ELD buffer sort of work >> >> drivers/gpu/drm/i915/display/g4x_dp.c | 2 + >> drivers/gpu/drm/i915/display/g4x_hdmi.c | 2 + >> drivers/gpu/drm/i915/display/intel_audio.c | 792 ++++++++++-------- >> drivers/gpu/drm/i915/display/intel_audio.h | 7 + >> .../gpu/drm/i915/display/intel_audio_regs.h | 88 +- >> .../drm/i915/display/intel_crtc_state_dump.c | 17 + >> drivers/gpu/drm/i915/display/intel_ddi.c | 2 + >> drivers/gpu/drm/i915/display/intel_display.c | 43 + >> .../drm/i915/display/intel_display_types.h | 2 + >> drivers/gpu/drm/i915/display/intel_dp.c | 4 +- >> drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +- >> drivers/gpu/drm/i915/display/intel_sdvo.c | 54 +- >> 12 files changed, 608 insertions(+), 409 deletions(-) >> >> -- >> 2.35.1 >
From: Ville Syrjälä <ville.syrjala@linux.intel.com> There was some discussion around ELD precompute, so I decided to have a quick look at hooking that up. Unfortunately the i915 audio code is a bit of a mess so ended up with a patchbomb of cleanups. Sorry about that. The actually interesting stuff is at the end of the series. The precumpute+readot+state checker is pretty self explanatory stuff for the most part. But I think we need to decide what to do with the hardware ELD buffer in general. It's totally busted atm on HSW (and I'd expecpt BDW as well), but we had no idea since we had no readout+state checker for it. So do we try to salvage it (I guess to mainly act as some kind of "did we enable audio correctly?" canary) or do we just stop programming it outright? And on which platforms could we do that? Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com> Cc: Takashi Iwai <tiwai@suse.de> Ville Syrjälä (22): drm/i915/audio: s/dev_priv/i915/ drm/i915/audio: Nuke leftover ROUNDING_FACTOR drm/i915/audio: Remove CL/BLC audio stuff drm/i915/audio: Exract struct ilk_audio_regs drm/i915/audio: Use REG_BIT() & co. drm/i915/audio: Unify register bit naming drm/i915/audio: Protect singleton register with a lock drm/i915/audio: Nuke intel_eld_uptodate() drm/i915/audio: Read ELD buffer size from hardware drm/i915/audio: Make sure we write the whole ELD buffer drm/i915/audio: Use u32* for ELD drm/i915/audio: Use intel_de_rmw() for most audio registers drm/i915/audio: Split "ELD valid" vs. audio PD on hsw+ drm/i915/audio: Do the vblank waits drm/i915/audio: Precompute the ELD drm/i915/audio: Hardware ELD readout drm/i915/sdvo: Extract intel_sdvo_has_audio() drm/i915/sdvo: Precompute the ELD drm/i915/sdvo: Do ELD hardware readout drm/i915/audio: Hook up ELD into the state checker drm/i915/audio: Include ELD in the state dump hax: drm/i915/audio: Make HSW hardware ELD buffer sort of work drivers/gpu/drm/i915/display/g4x_dp.c | 2 + drivers/gpu/drm/i915/display/g4x_hdmi.c | 2 + drivers/gpu/drm/i915/display/intel_audio.c | 792 ++++++++++-------- drivers/gpu/drm/i915/display/intel_audio.h | 7 + .../gpu/drm/i915/display/intel_audio_regs.h | 88 +- .../drm/i915/display/intel_crtc_state_dump.c | 17 + drivers/gpu/drm/i915/display/intel_ddi.c | 2 + drivers/gpu/drm/i915/display/intel_display.c | 43 + .../drm/i915/display/intel_display_types.h | 2 + drivers/gpu/drm/i915/display/intel_dp.c | 4 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 54 +- 12 files changed, 608 insertions(+), 409 deletions(-)