Received: by 2002:a05:6358:7058:b0:131:369:b2a3 with SMTP id 24csp6722245rwp; Tue, 18 Jul 2023 05:07:41 -0700 (PDT) X-Google-Smtp-Source: APBJJlHEK+0sWA+JDz6lLJLMNnS2wsaVpG4XgqXX2KF03FNMr+H2WOCP1VGu31tEJhio1O6f8TrX X-Received: by 2002:a17:906:225a:b0:98e:16b7:e038 with SMTP id 26-20020a170906225a00b0098e16b7e038mr15360512ejr.23.1689682061344; Tue, 18 Jul 2023 05:07:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689682061; cv=none; d=google.com; s=arc-20160816; b=T9XBKseg4DIuwjgNphduG9BaElO+NGBC2UYPjXIJVeunmyPf9cEJcbe8NKYbBDxmkF 6ARZmQwOESLO0bkQIVHwSoPNbJ3KY3rxQTlC1dQDKEGBKq4Ndc2i+pAUPo+5mlUutd/9 KJRalgQa9VCfJDByvmYWZWXQ8Jatf6wPcp6iz9Jj94ijZodw6gmwjrn/VjlNaOhvuWuy 2nofpsCLXjocek+PWbRcZe2SjuH+u8ave93c1GWwAhSQg6JfVF/uK35774l+MJc0gD8y ucK4AZZTHE2lk5ggzLcJTQppvgMQ+Mf+p7XnGus/YRcY35AFCLN47sxW405kVwjuwwlS CxFA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to :organization:from:references:cc:to:content-language:subject :user-agent:mime-version:date:message-id:dkim-signature; bh=k7B6PCIdY8ACsr14TtByzzfYhAcNdAsz5seC9O5tb7Y=; fh=dGffI4KuijwX5YenNYHa3MeZzmWWZnm8XetFPWEhn8I=; b=Yrg/TpzQzjdcuBxtf6M2eEh/0PQJdjaLeDv55d2IxgHH2O5JdebJUfLlvv1y6gh2gy Bh8FgNhQb1HhZY3NvKqUxTPEis+6M4vJpogeqVHjuunlaJ8MdMfiLW3stG3+8T1RMf3k FOAEjdHl5Is0Ngd4DQWfBSZZAbGyuhWlw9DrZw5+SNl17nJ9LbliWeicubcUac/Dzlj2 2EanYLJhqHTAG8DUdZDF2ZJ4PN2XGcwp1c8qTJDq7e3D0FT1ltTe0Uw84Nq3Ivl4SRuY QMWUrXa6WLPIr9swhinrSq52ny4KR4tQvVSxxu7SDR/9cmhi75FuB2P3LoVcnJV7oSY6 DG0A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=e19dbwZv; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id i16-20020a1709064ed000b00993a4cd974esi1032756ejv.287.2023.07.18.05.07.09; Tue, 18 Jul 2023 05:07:41 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=e19dbwZv; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232072AbjGRL2y (ORCPT + 99 others); Tue, 18 Jul 2023 07:28:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42876 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231685AbjGRL2u (ORCPT ); Tue, 18 Jul 2023 07:28:50 -0400 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 28048E75; Tue, 18 Jul 2023 04:28:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689679729; x=1721215729; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=eG9i8FoJlSyTJM2sgpvziVRwaPmDaVtbaL3UEyeheKw=; b=e19dbwZv+EPy5/QVk7l2OM+OugGEdf17yR54Q4N7pVX+S2Fts2fXkmeh +qdb/WnCENniETqVe1EK3QtewJDcP/uGiRaZ1SDVClHFpX5RkdW7bIWvk sF0yQKV0kE32ED6mtBnpMIFho8eQLKBRd2RZIR9F0xrzoXZOI4WWnfPkN LozWjQCRr5/c88Ywef0eExV56w9Klay/XDTSs612B0Cgcieqh5Ek4oHr5 f4GOg4dOPFlBrUe1grqKwVLaCOJDMRaxcsTDVDrzcm6Ob9JttKTkaGSt6 OEuf0p1t/ehzvbvlKeYc3L7MsddZoLgiuVjUV95utvQSEtSQXfHOOrG3i g==; X-IronPort-AV: E=McAfee;i="6600,9927,10774"; a="368821003" X-IronPort-AV: E=Sophos;i="6.01,214,1684825200"; d="scan'208";a="368821003" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2023 04:28:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10774"; a="813719404" X-IronPort-AV: E=Sophos;i="6.01,214,1684825200"; d="scan'208";a="813719404" Received: from ahajda-mobl.ger.corp.intel.com (HELO [10.213.19.167]) ([10.213.19.167]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2023 04:28:42 -0700 Message-ID: <3c7dfc18-539f-2b0c-0c77-48b89ef96560@intel.com> Date: Tue, 18 Jul 2023 13:28:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.13.0 Subject: Re: [PATCH] drm/i915/tv: avoid possible division by zero Content-Language: en-US To: Su Hui , Dan Carpenter Cc: jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, tvrtko.ursulin@linux.intel.com, airlied@gmail.com, daniel@ffwll.ch, nathan@kernel.org, ndesaulniers@google.com, trix@redhat.com, ville.syrjala@linux.intel.com, mripard@kernel.org, ankit.k.nautiyal@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, llvm@lists.linux.dev, kernel-janitors@vger.kernel.org References: <8e3e945b-9cae-71e9-fc68-dc1c64e5ea86@nfschina.com> From: Andrzej Hajda Organization: Intel Technology Poland sp. z o.o. - ul. Slowackiego 173, 80-298 Gdansk - KRS 101882 - NIP 957-07-52-316 In-Reply-To: <8e3e945b-9cae-71e9-fc68-dc1c64e5ea86@nfschina.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18.07.2023 12:10, Su Hui wrote: > On 2023/7/18 13:39, Dan Carpenter wrote: >> On Mon, Jul 17, 2023 at 04:52:51PM +0200, Andrzej Hajda wrote: >>> >>> On 17.07.2023 08:22, Su Hui wrote: >>>> Clang warning: drivers/gpu/drm/i915/display/intel_tv.c: >>>> line 991, column 22 Division by zero. >>>> Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1, >>>> then division by zero will happen. >>>> >>>> Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation") >>>> Signed-off-by: Su Hui >>>> --- >>>>    drivers/gpu/drm/i915/display/intel_tv.c | 3 ++- >>>>    1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c >>>> b/drivers/gpu/drm/i915/display/intel_tv.c >>>> index 36b479b46b60..82b54af51f23 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_tv.c >>>> +++ b/drivers/gpu/drm/i915/display/intel_tv.c >>>> @@ -988,7 +988,8 @@ intel_tv_mode_to_mode(struct drm_display_mode >>>> *mode, >>>>                  const struct tv_mode *tv_mode, >>>>                  int clock) >>>>    { >>>> -    mode->clock = clock / (tv_mode->oversample >> >>>> !tv_mode->progressive); >>>> +    mode->clock = clock / (tv_mode->oversample != 1 ? >>>> +            tv_mode->oversample >> !tv_mode->progressive : 1); >>> Seems too smart to me, why not just: >>> mode->clock = clock / tv_mode->oversample; >>> if (!tv_mode->progressive) >>>      mode->clock <<= 1; >> This is nice. > > mode->clock = clock / tv_mode->oversample << !tv_mode->progressive; > > But I think this one is much better,  it has less code and run faster. > Should I resend v3 to add some explanation or follow Dan's advice? Speed gain here is irrelevant here, and disputable. One thing which could be problematic is that we could loose the least significant bit in mode->clock, in case non-progressive, but I am not sure if it really matters, as mode->clock is not precise value anyway. Alternatively we could 1st shift, then divide, but in this case overflow can occur, at least in theory - I suspect there are no such big clocks (in kHz). Finally I would agree with Dan, readability is better with conditional. Regards Andrzej > > Su Hui > >> regards, >> dan carpenter