Re: [PATCH] CHROMIUM: drm: bridge/dw_hdmi: Eliminate unused cable_plugin

[Russell King - ARM Linux]

On Tue, Sep 29, 2015 at 01:07:25PM +0200, Thierry Reding wrote:
> On Fri, Sep 25, 2015 at 10:29:51AM +0200, Philipp Zabel wrote:
> > Am Montag, den 21.09.2015, 15:15 +0100 schrieb Russell King - ARM Linux:
> > > On Mon, Sep 21, 2015 at 11:51:06AM +0200, Thierry Reding wrote:
> > > > On Wed, Sep 16, 2015 at 01:41:38PM -0700, Douglas Anderson wrote:
> > > > > There's a member in 'struct dw_hdmi' called cable_plugin.  It's never
> > > > > set to anything anywhere so thus is always false.  There's a bit of code
> > > > > checking it, but since it's always false this must be dead code.
> > > > > Eliminate it.
> > > > > 
> > > > > Note: if someone wants to figure out the intention of the original code
> > > > > and implement whatever feature / fix was needed then we can drop this
> > > > > patch.  The 'cable_plugin' member has been unused since the code was
> > > > > first added in (9aaf880 imx-drm: Add mx6 hdmi transmitter support).
> > > > > 
> > > > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> > > > > ---
> > > > >  drivers/gpu/drm/bridge/dw_hdmi.c | 9 ---------
> > > > >  1 file changed, 9 deletions(-)
> > > > 
> > > > Except for the CHROMIUM: prefix this looks good to me:
> > > > 
> > > > Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>
> > 
> > This seems to be similar to Sascha's "drm: bridge/dw_hdmi: remove unused
> > code" patch, except that the hdmi_disable_overflow_interrupts function
> > could be removed too.
> > 
> > > > Russell, do you have patches to this driver queued for v4.4 and plan to
> > > > pick this up into your tree or should I take it?
> > > 
> > > My current patch stack for imx-drm related stuff looks like this at
> > > present:
> > >
> > > drm: bridge/dw_hdmi: place PHY into low power mode when disabled
> > > drm: bridge/dw_hdmi: start of support for pixel doubled modes
> > > drm: bridge/dw_hdmi: remove CEC engine register definitions
> > > drm: bridge/dw_hdmi-cec: add Designware HDMI CEC driver
> > > cec: add HDMI CEC input driver
> > > cec: add HDMI CEC core driver
> > > drm: bridge/dw_hdmi: replace CTS calculation for the ACR
> > > drm: bridge/dw_hdmi: remove ratio support from ACR code
> > > drm: bridge/dw_hdmi: adjust pixel clock values in N calculation
> > > drm: bridge/dw_hdmi: avoid being recursive in N calculation
> > > drm: bridge/dw_hdmi-ahb-audio: allow larger buffer sizes
> > > drm: bridge/dw_hdmi-ahb-audio: basic support for multi-channel PCM audio
> > > drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver
> > > drm: bridge/dw_hdmi-ahb-audio: add audio driver
> > > drm: bridge/dw_hdmi: improve HDMI enable/disable handling
> > > drm: bridge/dw_hdmi: add connector mode forcing
> > > drm: bridge/dw_hdmi: add support for interlaced video modes
> > > gpu: imx: fix support for interlaced modes
> > > gpu: imx: simplify sync polarity setting
> > > 
> > > I haven't yet decided what, if anything, from that stack I'm going to
> > > try to get into the next merge window.  Given the lack of interest last
> > > time I posted these patches, I'm loosing interest myself in trying to
> > > get them merged, especially ones which are getting on for being 2 years
> > > old.
> > 
> > I'm still very interested to see at least the "gpu: imx: fix support for
> > interlaced modes" and "gpu: imx: simplify sync polarity setting" merged.
> > May I take them into the imx-drm tree separately?
> 
> The "gpu: imx:" patches sound like they are standalone, so taking them
> through the imx-drm tree would be the easiest.

Taking just those screws up the rest of the series, as I previously
explained:

> On Thu, Aug 27, 2015 at 10:39:12AM +0200, Philipp Zabel wrote:
> > Unfortunately these timings are completely different from what Freescale
> > came up with for the TV Encoder on i.MX5, but the code we have currently
> > in mainline doesn't work for that either. I suppose I'll follow up with
> > a patch that adds yet another sync counter setup for the i.MX5/TVE case.
> >
> > I'd like to take the two ipu-v3 patches, making a few small changes on
> > this one:
> 
> Please don't split the series up.  The reason it's a series is because
> there's interdependencies between the patches.

Have you acked my patch set sent in August, after I relied saying that
the series needs to be kept together?

Philipp sent a tested-by, but that was about the only attributation
I received from posting the set, so I only sent David what I was fully
happy to send at the time.  I'm willing to send:

drm: bridge/dw_hdmi: improve HDMI enable/disable handling
drm: bridge/dw_hdmi: add connector mode forcing
drm: bridge/dw_hdmi: add support for interlaced video modes
gpu: imx: fix support for interlaced modes
gpu: imx: simplify sync polarity setting

now, which are patches 3, 4, 5, 11 and 12 from the original 12 patch
series.

The actual audio patches I think need to be held _even_ longer, because
it's unclear what's happening with ALSA in regard to HDMI audio.  There's
also the need to sort out what's going on with CEC (thanks to the late
arrival of the CEC project by Hans) which may influence how the HDMI
audio part gets to hear about the display's capabilities.  So, yet again
I can see audio missing the next merge window, which is just how it's
been for the last two years.

Actually, right now I think we ought to say: sod it, these patches have
been around for long enough that they should just be merged, and then
we'll sort out whatever changes are necessary later when the CEC and
audio situation finally gets sorted... but are you happy with having
what is essentially an ALSA driver in drivers/gpu/drm/bridge?  If not,
why haven't you responded to the many times that the patches have been
posted?

At this stage, I'm getting to be very rebelious, and it's very tempting
for me to say "sod you" or just drop the work entirely if you have any
objections.  I feel I've put fsck loads of work into this area for
very little mainline progress, and I'm just not going to continue doing
that.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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/