Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751972AbdGMMZD (ORCPT ); Thu, 13 Jul 2017 08:25:03 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35561 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751165AbdGMMZB (ORCPT ); Thu, 13 Jul 2017 08:25:01 -0400 Date: Thu, 13 Jul 2017 14:24:54 +0200 From: Daniel Vetter To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: =?iso-8859-1?Q?St=E9phane?= Marchesin , David Airlie , intel-gfx , Linux Kernel Mailing List , "dri-devel@lists.freedesktop.org" , Michael Davidson , Grant Grundler , Matthias Kaehlcke , Greg Hackmann , Daniel Vetter Subject: Re: [Intel-gfx] [PATCH RESEND] drm/i915: Fix pipe/transcoder enum mismatches Message-ID: <20170713122454.jmhqlxl4z76qdf6g@phenom.ffwll.local> Mail-Followup-To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , =?iso-8859-1?Q?St=E9phane?= Marchesin , David Airlie , intel-gfx , Linux Kernel Mailing List , "dri-devel@lists.freedesktop.org" , Michael Davidson , Grant Grundler , Matthias Kaehlcke , Greg Hackmann , Daniel Vetter References: <20170420215605.176722-1-mka@chromium.org> <20170505172636.GA128305@google.com> <20170505174043.GK12629@intel.com> <20170713101351.GS12629@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170713101351.GS12629@intel.com> X-Operating-System: Linux phenom 4.11.0-1-amd64 User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2796 Lines: 60 On Thu, Jul 13, 2017 at 01:13:51PM +0300, Ville Syrj?l? wrote: > On Wed, Jul 12, 2017 at 07:28:14PM -0700, St?phane Marchesin wrote: > > On Fri, May 5, 2017 at 10:40 AM, Ville Syrj?l? > > wrote: > > > > > > On Fri, May 05, 2017 at 10:26:36AM -0700, Matthias Kaehlcke wrote: > > > > El Thu, Apr 20, 2017 at 02:56:05PM -0700 Matthias Kaehlcke ha dit: > > > > > > > > > In several instances the driver passes an 'enum pipe' value to a > > > > > function expecting an 'enum transcoder' and viceversa. Since PIPE_x and > > > > > TRANSCODER_x have the same values this doesn't cause functional > > > > > problems. Still it is incorrect and causes clang to generate warnings > > > > > like this: > > > > > > > > > > drivers/gpu/drm/i915/intel_display.c:1844:34: warning: implicit > > > > > conversion from enumeration type 'enum transcoder' to different > > > > > enumeration type 'enum pipe' [-Wenum-conversion] > > > > > assert_fdi_rx_enabled(dev_priv, TRANSCODER_A); > > > > > > > > > > Change the code to pass values of the type expected by the callee. > > > > > > > > > > Signed-off-by: Matthias Kaehlcke > > > > > --- > > > > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > > > > > drivers/gpu/drm/i915/intel_dp.c | 6 ++++-- > > > > > drivers/gpu/drm/i915/intel_hdmi.c | 6 ++++-- > > > > > drivers/gpu/drm/i915/intel_sdvo.c | 6 ++++-- > > > > > 4 files changed, 14 insertions(+), 8 deletions(-) > > > > > > > > Ping, any comments on this patch? > > > > > > I'm not convinced the patch is making things any better really. To > > > fix this really properly, I think we'd need to introduce a new enum > > > pch_transcoder and thus avoid the confusion of which type of > > > transcoder we're talking about. Currently most places expect an > > > enum pipe when dealing with PCH transcoders, and enum transcoder > > > when dealing with CPU transcoders. But there are some exceptions > > > of course. > > > > > > I don't follow -- these functions take an enum transcoder; what's > > wrong about passing what they expect? It seems like what you are > > asking for has nothing to do with the warning here... > > There's a warning? I don't get any. > > Anyways, I just don't see much point in blindly changing the types > because it doesn't actually solve the underlying confusion for human > readers. It might even make it worse, not sure. Yeah the current patch just makes it worse. Either enum pch_transcoder, or the enum pipe changes I suggested somewhere else. Blindly fixing type mismatches without actually fixing the underlying confusion isn't really useful work. And at least the switch to enum pipe for pch won't be (much) bigger. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch