From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755606AbdEEUJ5 (ORCPT ); Fri, 5 May 2017 16:09:57 -0400 Received: from mga01.intel.com ([192.55.52.88]:45262 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755478AbdEEUIV (ORCPT ); Fri, 5 May 2017 16:08:21 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,294,1491289200"; d="scan'208";a="83152780" Date: Fri, 5 May 2017 23:08:10 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Grant Grundler Cc: Matthias Kaehlcke , Daniel Vetter , Jani Nikula , David Airlie , intel-gfx@lists.freedesktop.org, LKML , Greg Hackmann , Michael Davidson , dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH RESEND] drm/i915: Fix pipe/transcoder enum mismatches Message-ID: <20170505200810.GL12629@intel.com> References: <20170420215605.176722-1-mka@chromium.org> <20170505172636.GA128305@google.com> <20170505174043.GK12629@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 05, 2017 at 12:12:49PM -0700, Grant Grundler 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. > > Is an enum better than coding an explicit conversion in an inline function? The point of the enum would be to make it more clear which piece of hardware we're talking to in each case. But this would require going through the entire PCH code and changing things to use the right type in each case. Quite a bit of work with little measurable gain I'd say. > Then the code can do some sanity checking as well. Something like: > > enum transcoder pch_to_cpu_enum(enum pipe) > { > WARN_ON(pipe > FOO); > return (enum transcoder) pipe; > } That would have to be called pipe_to_pch_transcoder() or something like that. > > > 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. > > cheers, > grant > > > > -- > > Ville Syrjälä > > Intel OTC -- Ville Syrjälä Intel OTC