Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756495AbcLSPzI (ORCPT ); Mon, 19 Dec 2016 10:55:08 -0500 Received: from mail-cys01nam02on0072.outbound.protection.outlook.com ([104.47.37.72]:15995 "EHLO NAM02-CY1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756367AbcLSPyg (ORCPT ); Mon, 19 Dec 2016 10:54:36 -0500 Authentication-Results: spf=pass (sender IP is 149.199.60.83) smtp.mailfrom=xilinx.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=bestguesspass action=none header.from=xilinx.com; X-IncomingTopHeaderMarker: OriginalChecksum:;UpperCasedChecksum:;SizeAsReceived:2964;Count:27 From: Appana Durga Kedareswara Rao To: Jose Abreu , "dan.j.williams@intel.com" , "vinod.koul@intel.com" , "michal.simek@xilinx.com" , Soren Brinkmann , "moritz.fischer@ettus.com" , "laurent.pinchart@ideasonboard.com" , "luis@debethencourt.com" CC: "dmaengine@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma Thread-Topic: [PATCH 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma Thread-Index: AQHSVuWKeAv0STYsN0iupc5V13K6T6EIp48AgACywOCAAHtTAIADe3AQ Date: Mon, 19 Dec 2016 15:40:02 +0000 Message-ID: References: <1481814682-31780-1-git-send-email-appanad@xilinx.com> <1481814682-31780-3-git-send-email-appanad@xilinx.com> <9d92984b-e04a-cd29-e933-d8ea4d610c94@synopsys.com> <689b1077-6ee3-60d0-1fdf-0a125003a479@synopsys.com> In-Reply-To: <689b1077-6ee3-60d0-1fdf-0a125003a479@synopsys.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.23.229.139] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-RCIS-Action: ALLOW X-TM-AS-Product-Ver: IMSS-7.1.0.1224-8.0.0.1202-22770.006 X-TM-AS-User-Approved-Sender: Yes;Yes X-IncomingHeaderCount: 27 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-Forefront-Antispam-Report: CIP:149.199.60.83;IPV:NLI;CTRY:US;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(7916002)(39850400002)(39840400002)(39450400003)(39410400002)(39860400002)(2980300002)(438002)(189002)(51914003)(199003)(81156014)(2950100002)(8676002)(189998001)(54356999)(626004)(2920100001)(33656002)(5660300001)(97756001)(7696004)(6116002)(76176999)(2201001)(5001770100001)(102836003)(2900100001)(50986999)(8746002)(81166006)(3846002)(23726003)(50466002)(2501003)(5250100002)(2906002)(92566002)(63266004)(8936002)(7736002)(106466001)(305945005)(47776003)(46406003)(93886004)(356003)(229853002)(4326007)(55846006)(106116001)(38730400001)(107986001);DIR:OUT;SFP:1101;SCL:1;SRVR:BY1PR0201MB1077;H:xsj-pvapsmtpgw01;FPR:;SPF:Pass;PTR:unknown-60-83.xilinx.com;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;CY1NAM02FT043;1:Q4l1GIY5hL32saWaf2W4jP2jvak2nyQwJRYo2+3nYxY/w+xXH+8dLTAj99tV5239eEzKeSPkvNsxYPDnYbtC8u/AnI+/BLDF3wBsPlXVX5AAIA+FCx+f1wafV6CjO7Sytf9IBcOy3tZ+D0T04jXagqdxAQUQwfEGf5LbCsaRbLaIHgEZzRDJo67s/Wlu9YVY16k9nUi4VWtJBKfeh+CAzwTwpkl7X9T0U2uAX9KEgtfGLiMW3roOoE3CHGPE+qckwjZE/iregMG7ICmz0EWuEiBHf+gtv3dJyDU3/+JiQyL0Whod4whUwIHj6cwEyCa8Q88RGEmc3lNPwSJ+WtckgTOykLVpizz+JkzGPexV1royHNCcxTPBB0KWTAH74xbYRacl5opRhdttyeTY1MJloHfAWFP/srnkXhEti0+0hLU3GiFDMQLKeb6qsA7bgw1U10I1tmppYnbf4Xk+R+Jf5zhWphpDcLmnrGNUxBh5Jvc3pmeIGF1OAGcOq9Epb1MdnRTiXS/7r7B17xqDyyp1an0G9kqTbhv5oLW6xy9NemQx+QPOifK6sHbGy4ccerSXp0b2cAPJwEVXzCoTQmwFvxvdxlIm2ZMTH+9VLZrmZr1/BYNgpjBkEilSujsD6ee8pM/KaNFyfcmgDs9MbPGTqg== X-MS-Office365-Filtering-Correlation-Id: b0c8d875-80ef-4fa9-17e6-08d428254fc0 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(8251501002);SRVR:BY1PR0201MB1077; X-Microsoft-Exchange-Diagnostics: 1;BY1PR0201MB1077;3:NWdGYgszgZM20WdlzCFggc6bZijpZjePSYE9kWAJ86MQ5fxlBgIHopBSeNIecK14RKdK8IfYtKlDxG79fo8+nUHYxBL888a/9Zzhkwb3NzYEdIjU33LiwBDp8g7+CElTl+94JeXqdQMa92lNg0H6TmhQ+BzG+Luxj87TLza71aC8iwUex3bgrUxEmhji3nV7bcmsM31dCBZ76gFnVUUahVVS4FAyL6gPPm+HNcD9FfSLUkiMFbOLjLP4DUTBvzDjcMmTlNuHaJ47/DxaXvq3f3p9yg0TT8G6CbxdwDJi/YI5eu6l6bdzKsWceAOqoeNxExXu3V87qG3f+usZYB534+ig9jiLP2RRNDgxgLAgiviH1LEhn1XdYCpW/2SPmk7u4EvRfZ7uVpfTZfvAYGYoWA==;25:3TBvNcDlfZ3UCdzpfgKMgaeAuR9Xqkw9O6FWtyFOiKT/Y12ilWNmsbHNBkyGEyCOWxJLf2kPQFdxGv46Gr1vRpbJSPTnXRv8cXE+X0iXYyqmU98w0K3CUNcLUrJbY7VFa4tUiEvEdmZesADDXw0mfzrFZGJxVubwBsb877Z2kmtckAK7iy6AWF6s+tJyKIzAadjyeErESVCKz6wp43/KVqyYN9jBk9pJEW8ZIPBgCm4P738RGFjc6NjkPr/Aw4OPY1Jg9htedCuMWvy8DfKgJ09Nzx2fKaah+ZNdp4xOLeSSKkuLZAFXnF+BnF6UG4PUbkY0rjiEHhMDS4DuRJkdPwcYt3G5J9onv/6WkqOZh4rznFHYp4rfCxtq1jlyY1MSb/RRh4A0hwSEbdiIPaD2beZfSyX8v6MUM1dz0LOjmgWlaowpL6k9E3h/qBu/3zyMEloNperlDKjQf9icgrO3bQ== X-Microsoft-Exchange-Diagnostics: 1;BY1PR0201MB1077;31:/qXqTen3Cr7plveoiA6+db/omuEsprJJkli9epI2A9eGAXrMDfWsqjGiQ/beZ0JPadBWxC94m0fnIg30kHXZywu27WZRfGL79NBgsQ/a8Aw9s13FUYU+ZMvLKlKOhNp/Lk7Mjo5b2rlujDnLF7hvK+PFkfBifp9YWWCu3N09UKm7dgOJ+4xwRw4R6MenksD6lwz0GHyl9UnkE7NmQocFLBc801C75F+7N7w24VbUqSzCXx5hEzPjIv3w44AWbSkluKiMqr0Icc1ysZRSGRpymg==;20:bmIJVAyC7VLVtWMVc8jMTbqpcqntSjp6yadJKRkbgHLkgyHF7Lp7bhtooUMuDV/xrBxhX3qX8cC/Asr+lpnJ6L+r420PRIMK85P6STKXZ0gQaXbQ9T7d2aDGTvSlph9a810DjIZHPiSORiqdmnNhV1etN/KxLB0zGyjcMJRtGTTNp/HPoahHyBIl6jdNEz6tHmQp1sVaFtak4IgPLkBMjiNE3thzRsAEAJN3bqwraEVxN4zYBULzgJG+T73N3sdhuvScjoEL23n0zZBL6+Blum0FuhHZ+hVGexRY/tQ8C4oYqIwJFCMiXSFvkLcXY6i4gkxi9S5KACLeTznlgwWQ6x8E4N6pUy8DEhLt4oga+k6fWdiK5jtvn8XxfV9Jg1BRReeuzF5ge3VFICTJJvLnqTO1xtC6jh+Z8WjQ8VgXjb3EVP8061FwUC4bsFiMtRHC0qJi5eDyDvAOjPxGzIhxhzPAYYEJvZf7iX/OlZSUolvYc4hXbk6G28q8xVYaDhYu X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(17755550239193); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(13023025)(13024025)(13015025)(13017025)(13018025)(3002001)(10201501046)(6055026)(6041248)(20161123555025)(20161123560025)(20161123564025)(20161123562025)(6072148);SRVR:BY1PR0201MB1077;BCL:0;PCL:0;RULEID:;SRVR:BY1PR0201MB1077; X-Microsoft-Exchange-Diagnostics: 1;BY1PR0201MB1077;4:dWhUmtwK0x/va/18tNMmF/IJmneexweigCQKWQfjsejmPe8/a2eJ3hZzRJpdo4u7OPuzl1gJMevDFUdTQzPDuT8wqgsFDJue3frakGakmzNYGHk5Ue5YzT27MUoen5UuHdeEJ6tPaSnEJlkqLmR7I1kXjzkifbV0hdJaMtEoAGVMcENz4FVs9aAf+qZPkLTBNBPaV7YODPtqeeF/eiUgSV3kpJQQ1CCF7L0sG6VMSYSIseeekJbPh1R4wyJ31FzY4PEqudRHyQSMi8n+rzuXIGAjg7gIaSJqzSwRaUjOpi+Fk1eci91V7lyCiCDjDS0ckBHCvn7ZccOayaW35yJN+COTmcgxKzoJhclu8caLGbAHb5BC19/B56ay/vi/IwrR7pLko3pG1luMvqETJqG3PScF8ZYUWdd4KxgEw8iNj4srdlXoi27sfe7QDCUKXVFWP9Gdo9om+zjbJSF3VNxmPqPRI1MTen3IFDfkH5wjNcC0aqdtWZcB/CP+UJa2sQU6DaXJk21KkGxpjuc75x1ojKaXLShfTP6uIg7m1cuigQT6Lka6/WJPvXTCYICYm72XPdvxWP655F6fZPZKhnNf3SVmGcRS4t+/9AU2oWztPIFb0t9SQ/nvd40A9kzbmTX/qhJ/bulO1pbWAySqnr4gLbANQTfoNIsYV5CUBCOvq/RIaGXGheXZtcJLG1U+i5AkGpA/wfqIy9Yx+niDmF6IoRHanhoSrE+gqpy5Z2cIFdo= X-Forefront-PRVS: 01613DFDC8 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BY1PR0201MB1077;23:To4croVgAK7+rTJ/+BoG/17OuoRZLQMA6e61wNh?= =?us-ascii?Q?UTb8b56WCKrt/ZkrkVLC3AVQNGVPO44MqOAFB7+zILwSIg/ChGcMrLvXKEG3?= =?us-ascii?Q?znDbh+uGXzpzxSnpNcs80U67Ugttr3b2PcgiAFJ+KiP2plFDwZDEx5k4uVyG?= =?us-ascii?Q?XuBQ6oM43+Jbtct3A09M9juR4XVBHHe8ifCaiUjeEiHoEmMWON7q9WgY6nCH?= =?us-ascii?Q?Y7Fzun6/p6qUvl+ACOVo7vNLAlyCLbqtBQYR0ywErqPR1A0IXLdTYRCfmjtX?= =?us-ascii?Q?LHAZ3ILQYP9qxy5osAZyYFMAi5QYM3qF0JeODTkwZtdfQHHq1ZH/rgYy6ocm?= =?us-ascii?Q?rCv5qtKg9fUZUG/GL4hB+tgEQTWfHchfQlCUNxJH6NM0EVjJ0KSwGyeDaO2A?= =?us-ascii?Q?IT1cNRbukxvq44LOrPx5LlPxRue8BF5ltjxfPI7aF95abApsuYBFjguZUPXB?= =?us-ascii?Q?d99XJfqnbIOqqy68JVZCg41rEeqQoU903tgA7u2fbsPIZqykcgi8OEWzMoR4?= =?us-ascii?Q?ZMbranikR7swjLf/9zV7D+VUb2hAQ73iucRJbyHqTZqjll1ZHTHrL7M6DNUP?= =?us-ascii?Q?k7wsspsJNk5PKqB037uoeYi/Gl7r+n5XxYnBl+NID5iJK3aVXB3RbIMut28Q?= =?us-ascii?Q?R+OfBwiR6JOdy9Gltf3ICbe0CjQ0ZgPK6ts6DoJz487POeqQD7wbTgKTM7we?= =?us-ascii?Q?bw8L78/5+ug9n+a8O/XQS/wnAJqu4mgQ1skLlnzF4SAwGgIvG40pOfWRB9IG?= =?us-ascii?Q?3K1ECZIDQ7zYYNBrHOhm/7E1g54NxIbFI0Kg6tvPIfVtt3qDBygVGVtVeyOh?= =?us-ascii?Q?/ZtAmlufwDi6SSEyCk44ic1nb189TGSFG3ljOpHcIUk2JHp4vrPZiJYxxIb7?= =?us-ascii?Q?3H1L9Fz/+WCzbyCrmjjaR/SbE2TRjSH2q585DLPutI9kdRjyzD1+SEs4ERLy?= =?us-ascii?Q?4bQOcfEuyBT0rfeLPnLDo4yj96KGhWqRLnUo6pW25alhH3JQTvJJrpRPHIB1?= =?us-ascii?Q?tHv9CjAchQZ3TLFWrFPIJ1V870djP05D/VfKy54yD7+aAzSWrj8fYGg5ZhnP?= =?us-ascii?Q?e66u0ZeX6Mew3SxDDXZNBfjAy6tpI+ZdL7ir7HivYz6r7WZemP7ne68LGh5y?= =?us-ascii?Q?5CWWLYCAlZW+w4PQ7Q99B+j90suAH4zctTncAHpVPlHTlfRIAMkzcotCNP73?= =?us-ascii?Q?BAvoEsxUCn3IFArSuD/WBOMo/DuUoaiM6nj1bDOj605h9C4mzEZzMRkRgp0W?= =?us-ascii?Q?7TDXniPtJnbLM7w96mfghGCP5qQ2GbljnJ1wncAsPbLLUU8RsaAYOKXpVRAU?= =?us-ascii?Q?EdzQ4UsTQVKMbFjYomVNqkXc=3D?= X-Microsoft-Exchange-Diagnostics: 1;BY1PR0201MB1077;6:VVGySfqrq+sU38df/9Wpq18MGZ7xVMf8+XHQEW3+J/sizjg1XWlWqIa5/zT47LvP0z5mzqI3e6zwPSE+1t+JGp9AUZk/Fl5zbSW+gUBt36fvNdq9oIUEBYidtucznwhYhs/G+GzHAf3xJA2nOeMKmjl+lGZVRNGa6SSL7L8+jYNUn6ioJiDcTwochrfttcvhgxqle9396oMg6A471fIZTTIasVV80pQsO/cdgDrxARXbPoAVW/oDAzYXkXDA4bfUEnJeQYr5Y3B5sMP5EKz4hEoMxz1XNH2NOY8Mqd5iMUsihgx4o+od4Vf53HYRFxljcTIMtg/m/OwwxUZbauEZtG3jOjvAQfVmKX6VNlTM4rxGn9kdxoMHGdFEAIQZlZ5EUyZcN1nqdtxx6fxf6M/gDilfbYfW2PEaQGIA/Gj2qFe7TjkxJtoii5mfUHgGiZXsphUzt7T1/om3LPmnfoNE0Q==;5:Hqbjmde6SA/O3rf1P6uvcElTVr7OcwFYlT0bTwR/xXrfKNjnVMoumMHhtgaHJJhZT8c6SSJ9BPSnCgJByj1fgH2Cfvb4fOi+eZFNEA6489gpCN6bi1zclkVfC/CRfMsTkOjjHJt8xYfL5vNCEknDBw==;24:IIk2AvsR8xgzk6GBuTVHr4WLN+CYuL7ihev7YTJd+Tlz4x3yc/3e8Wzz+OhaLHHHbtg5HNjKUuU1paq6KRIMKJSU/Au9+1sVmW6vzG9vxJw= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;BY1PR0201MB1077;7:oCJdjh2SNc6ekKGw/EqDoL2idweH54RdmDPn2puCYmp2DOJr8bbGsfvkja1HJvhksBoIFh4QvLzfXW1DABxo0cZxEtjzDMfGf6bgdS4a8cYxPwKNBOKepLs1jyG+BfYV5WUmcJrlzv9z23TNGu7dSzD7VfRlX9u6xJRF+z66vUcCXJH/T8tw4fOth4HNyGZpdO0vN85ZrCqcV8vL/1mKhteHF2bp8HuN/mnITNy86IOdC9MT/patfNXuy5PG3HhYRwNw5VhN1e4zfWlCQ9hlh/305nzD6SF+xkpFw4F3C+85IbplTvyc5aiM0iSZgTcdyADhOZ9uqHQMrEG6dDHTqezy3vslXsEy97WEybeIbYPgEyFXwqlbv7z0jchZKoPJ1/cUSZFIzMj3r/LlqFTWdummIGO7kwZ30Qvk3xb1E09JmWXHjgVcg9DXpV4fB9uhHEVam+K0AmKSCd4abFmKVw== X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2016 15:40:07.6878 (UTC) X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.83];Helo=[xsj-pvapsmtpgw01] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR0201MB1077 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uBJFtNKg004698 Content-Length: 3277 Lines: 101 Hi Jose Miguel Abreu, Thanks for the review... > > > >>> - last = segment; > >>> + for (j = 0; j < chan->num_frms; ) { > >>> + list_for_each_entry(segment, &desc->segments, node) > >> { > >>> + if (chan->ext_addr) > >>> + vdma_desc_write_64(chan, > >>> + > >> XILINX_VDMA_REG_START_ADDRESS_64(i++), > >>> + segment->hw.buf_addr, > >>> + segment->hw.buf_addr_msb); > >>> + else > >>> + vdma_desc_write(chan, > >>> + > >> XILINX_VDMA_REG_START_ADDRESS(i++), > >>> + segment->hw.buf_addr); > >>> + > >>> + last = segment; > >> Hmm, is it possible to submit more than one segment? If so, then i > >> and j will get out of sync. > > If h/w is configured for more than 1 frame buffer and user submits > > more than one frame buffer We can submit more than one frame/ segment to > hw right?? > > I'm not sure. When I used VDMA driver I always submitted only one segment and > multiple descriptors. But the problem is, for example: > > If you have: > descriptor1 (2 segments) > descriptor2 (2 segments) > > And you have 3 frame buffers in the HW. > > Then: > 1st frame buffer will have: descriptor1 -> segment1 2nd frame buffer will have: > descriptor1 -> segment2 3rd frame buffer will have: descriptor2 -> segment1 > but, 4th frame buffer will have: descriptor2 -> segment2 <---- INVALID because > there is only 3 frame buffers > > So, maybe a check inside the loop "list_for_each_entry(segment, &desc- > >segments, node)" could be a nice to have. With the current driver flow user can submit only 1 segment per descriptor That's why didn't checked the list_for_each_entry for each descriptors... Hope it clarifies your query... > > > > >>> + } > >>> + list_del(&desc->node); > >>> + list_add_tail(&desc->node, &chan->active_list); > >>> + j++; > >> But if i is non zero and pending_list has more than num_frms then i > >> will not wrap-around as it should and will write to invalid framebuffer > location, right? > > Yep will fix in v2... > > > > If (if (list_empty(&chan->pending_list)) || (i == chan->num_frms) > > break; > > > > Above condition is sufficient right??? > > Looks ok. Thanks... > >>> @@ -1114,14 +1124,13 @@ static void > >>> xilinx_vdma_start_transfer(struct > >> xilinx_dma_chan *chan) > >>> vdma_desc_write(chan, XILINX_DMA_REG_FRMDLY_STRIDE, > >>> last->hw.stride); > >>> vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last- > hw.vsize); > >> Maybe a check that all framebuffers contain valid addresses should be > >> done before programming vsize so that VDMA does not try to write to > >> invalid addresses. > > Do we really need to check for valid address??? > > I didn't get you what to do you mean by invalid address could you please > explain??? > > In the driver we are reading form the pending_list which will be > > updated by pep_interleaved_dma Call so we are under assumption that user > sends the proper address right??? > > What I mean by valid address is to check that i variable has already been > incremented by num_frms at least once since a VDMA reset. This way you know > that you have programmed all the addresses of the frame buffers with an > address and they are non-zero. Ok Sure will fix in v2... Regards, Kedar. > > Best regards, > Jose Miguel Abreu >