Re: [PATCH] drm/i2c: Fix broken TDA998x audio (was: Re: [PATCH v25/8] drm/i2c: tda998x: add video and audio input configuration)

From: Russell King - ARM Linux
Date: Sun Sep 22 2013 - 17:54:14 EST


Ping?

On Mon, Sep 02, 2013 at 03:50:57PM +0100, Russell King - ARM Linux wrote:
> On Wed, Aug 14, 2013 at 09:43:30PM +0200, Sebastian Hesselbarth wrote:
> > From: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
> >
> > This patch adds tda998x specific parameters to allow it to be configured
> > for different boards using it. Also, this implements rudimentary audio
> > support for S/PDIF attached controllers.
> >
> > Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx>
> > Tested-by: Darren Etheridge <detheridge@xxxxxx>
> > ---
>
> It looks like there's been a bug introduced in this patch (which wasn't
> in my original).
>
> > @@ -445,8 +681,7 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
> >
> > switch (mode) {
> > case DRM_MODE_DPMS_ON:
> > - /* enable audio and video ports */
> > - reg_write(encoder, REG_ENA_AP, 0xff);
> > + /* enable video ports, audio will be enabled later */
> > reg_write(encoder, REG_ENA_VP_0, 0xff);
> > reg_write(encoder, REG_ENA_VP_1, 0xff);
> > reg_write(encoder, REG_ENA_VP_2, 0xff);
>
> I also disabled the writing to REG_ENA_AP in the DPMS off path as well,
> which clears this register.
>
> That seems to be missing from this patch, and it means that when the
> display is placed into DPMS-off mode, the audio inputs are disabled,
> never to be re-enabled. There is no need to disable the audio input
> in DPMS-off mode.
>
> 8<=============
> From: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
> Subject: [PATCH] drm/i2c: Fix broken TDA998x audio
>
> In patch "drm/i2c: tda998x: add video and audio input configuration" in
> its original version, disabling the audio input port was removed. The
> version which was submitted for merging had this change deleted, which
> results in audio being non-functional. Fix this by removing the write.
> While here, update the comment too.
>
> Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i2c/tda998x_drv.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 5742cfc..59878af 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -705,8 +705,7 @@ tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
> reg_write(encoder, REG_VIP_CNTRL_2, priv->vip_cntrl_2);
> break;
> case DRM_MODE_DPMS_OFF:
> - /* disable audio and video ports */
> - reg_write(encoder, REG_ENA_AP, 0x00);
> + /* disable video ports */
> reg_write(encoder, REG_ENA_VP_0, 0x00);
> reg_write(encoder, REG_ENA_VP_1, 0x00);
> reg_write(encoder, REG_ENA_VP_2, 0x00);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/