From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1A938C55ABD for ; Thu, 12 Nov 2020 08:08:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BFFE820870 for ; Thu, 12 Nov 2020 08:08:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="Di49O4ZS" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726107AbgKLIIm (ORCPT ); Thu, 12 Nov 2020 03:08:42 -0500 Received: from fllv0015.ext.ti.com ([198.47.19.141]:41330 "EHLO fllv0015.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725996AbgKLIIj (ORCPT ); Thu, 12 Nov 2020 03:08:39 -0500 Received: from fllv0034.itg.ti.com ([10.64.40.246]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 0AC88PTx103946; Thu, 12 Nov 2020 02:08:25 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1605168505; bh=kTmPS+14L43/mN4J8NF4b/8F8zr576pK0ujbi8KDnd8=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=Di49O4ZSj3WbRunCYxEc8afWOd55tiXH2Cbl2vQP37Sb5FT75DhahuBaCy7Qlkj/J 7F9zVEq5BQIyFkIbSKDN/cz8HdJM+RYlm7FeNqS4mxgkvWW6CuCC6u/zhPbG1rsDLx xIqPJizmEOLzNpDi8+93I/oBYL0LxwpzKKIwMgx0= Received: from DLEE111.ent.ti.com (dlee111.ent.ti.com [157.170.170.22]) by fllv0034.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 0AC88Pe7016167 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 12 Nov 2020 02:08:25 -0600 Received: from DLEE104.ent.ti.com (157.170.170.34) by DLEE111.ent.ti.com (157.170.170.22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3; Thu, 12 Nov 2020 02:08:25 -0600 Received: from fllv0040.itg.ti.com (10.64.41.20) by DLEE104.ent.ti.com (157.170.170.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3 via Frontend Transport; Thu, 12 Nov 2020 02:08:25 -0600 Received: from [192.168.2.6] (ileax41-snat.itg.ti.com [10.172.224.153]) by fllv0040.itg.ti.com (8.15.2/8.15.2) with ESMTP id 0AC88MKE014002; Thu, 12 Nov 2020 02:08:23 -0600 Subject: Re: [PATCH v3 30/56] drm/omap: dsi: move panel refresh function to host To: Laurent Pinchart CC: Sebastian Reichel , Nikhil Devshatwar , , , Sekhar Nori , Tony Lindgren , "H . Nikolaus Schaller" , Sebastian Reichel References: <20201105120333.947408-1-tomi.valkeinen@ti.com> <20201105120333.947408-31-tomi.valkeinen@ti.com> <20201109101003.GA6029@pendragon.ideasonboard.com> <6118c70e-6dc5-2d87-fc68-266cd3eeb66c@ti.com> <20201111155854.GH4115@pendragon.ideasonboard.com> From: Tomi Valkeinen Message-ID: Date: Thu, 12 Nov 2020 10:08:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201111155854.GH4115@pendragon.ideasonboard.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org On 11/11/2020 17:58, Laurent Pinchart wrote: >>>> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c >>>> index 030a8fa140db..1582960f9e90 100644 >>>> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c >>>> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c >>>> @@ -177,27 +177,6 @@ static int dsicm_get_id(struct panel_drv_data *ddata, u8 *id1, u8 *id2, u8 *id3) >>>> return 0; >>>> } >>>> >>>> -static int dsicm_set_update_window(struct panel_drv_data *ddata, >>>> - u16 x, u16 y, u16 w, u16 h) >>>> -{ >>>> - struct mipi_dsi_device *dsi = ddata->dsi; >>>> - int r; >>>> - u16 x1 = x; >>>> - u16 x2 = x + w - 1; >>>> - u16 y1 = y; >>>> - u16 y2 = y + h - 1; >>>> - >>>> - r = mipi_dsi_dcs_set_column_address(dsi, x1, x2); >>>> - if (r < 0) >>>> - return r; >>>> - >>>> - r = mipi_dsi_dcs_set_page_address(dsi, y1, y2); >>>> - if (r < 0) >>>> - return r; >>>> - >>>> - return 0; >>>> -} >>>> - >>> >>> I can't tell whether this is common to all command-mode panels, or if >>> there could be a need for panel-specific update procedures, so I can't >>> really ack this patch. >> >> I can't say either, but all the command mode panels I know need and support this. And, afaik, we >> have only the single cmd mode panel driver which we add in this series. > > Now that I think about it again, isn't it a layering violation ? > Shouldn't the DSI host handle DSI commands transfers, with the panel > driver taking care of specific DSI commands ? Well, the DSI host (the HW) already handles specific DSI commands, as it does the update with DCS Write Start/Continue commands. The update is initiated from omap_crtc, via dssdev->dsi_ops->update(). We could perhaps add a new function to drm_panel_funcs, say, prepare_update, which could then do the above. Although I think the above code is not strictly speaking required, as the panel should remember the column and page address, and as such, they could be set just once at config time. However, I remember debugging issues related to this. And with a quick test, I can see that things break down if I just do the above once in the panel's setup. But things work if I send a DSI NOP instead in the dsi host. So looks to me that either the OMAP DSI or the panel I have need some command transmitted there. It probably has to happen between two frame transfers. There are also other things related to update which I'm not so happy about: 1) the TE gpio irq is handled in the dsi host driver now, even if it's a panel gpio, 2) the dsi host driver snoops the DSI packets sent by the panel, and catches TEAR_ON/OFF packets, and then change internal state accordingly. So... I could change the dsi host driver to only send a NOP, and do the page/column call from the panel's setup. That simplifies the code. Or I could add the new function to drm_panel_funcs, and send a NOP from there. But if this "needs a NOP" is an OMAP DSI feature, the panel driver is not the right place for it. I also think that managing the TE cleanly needs more thought, and probably requires some more interaction between the dsi host and the panel. It might be better to look at both the update callback and the TE at the same time. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki