From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 01/10] drm/i915: Add automated testing support for Displayport compliance testing Date: Fri, 24 Oct 2014 10:24:23 +0200 Message-ID: <20141024082423.GV26941@phenom.ffwll.local> References: <1412869090-48010-1-git-send-email-tprevite@gmail.com> <1412869090-48010-2-git-send-email-tprevite@gmail.com> <544933C7.4070904@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f48.google.com (mail-wg0-f48.google.com [74.125.82.48]) by gabe.freedesktop.org (Postfix) with ESMTP id D63386E585 for ; Fri, 24 Oct 2014 01:24:14 -0700 (PDT) Received: by mail-wg0-f48.google.com with SMTP id k14so564066wgh.7 for ; Fri, 24 Oct 2014 01:24:13 -0700 (PDT) Content-Disposition: inline In-Reply-To: <544933C7.4070904@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Todd Previte Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Thu, Oct 23, 2014 at 09:58:47AM -0700, Todd Previte wrote: > On 10/20/2014 10:48 AM, Paulo Zanoni wrote: > >2014-10-09 12:38 GMT-03:00 Todd Previte : > >>+/* Displayport compliance testing - PHY pattern testing */ > >>+static uint8_t > >>+intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp) > >>+{ > >>+ uint8_t test_result = DP_TEST_NAK; > >>+ return test_result; > >>+} > >I guess a lot of people would have just made the code return NAK > >without even defining/calling these functions above. > > It came down to two ways of implementing this initial patch. I could have > NAK'd in the handler and omitted these functions until they were fully > implemented. Or I could do what I did here, which was to put the skeleton in > place and add the flesh to bones when ready. To me, this seems to make more > sense to me because it gets the structure in place and builds on it in > subsequent patches. Random drive-by comment: Imo rolling out the scaffolding in this patch here looks ok. Personally I'd probably have done what Paulo suggested too. Maybe even with a default switch that just yells with a DRM_ERROR("Test not yet implement: %i\n", testcode) and then fill things out step-by-step. But that's kinda a bikeshed ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch