From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 927921A0B08; Fri, 7 Feb 2025 20:12:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.168.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738959139; cv=none; b=rvTZ2mPehmwiJMi6070WEQWPBQ9QauN8n1isDBAGvGilhMzNmy8lN92ciEXkNHn4mN/bY2naG+CDTCKdgBBJKSecp99CYWG1Y8Iil4Te5YMzUJKMIjRDM4sjIVpQC3LI6AHgGA0W7ePX57wWtRkiSanchD0uFK5BByCCHDKc7zM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738959139; c=relaxed/simple; bh=js3/L1kH4kxEihA1SRO6/53bxBojLIfkR9/vdjllX8M=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=VGqEXrpu7sil8P7hYou1XEY4gUn6oNKCWlpI9AFBOGpEZmmE4MCnLy58D0FCTD7qrRdpXQvz3tBQPYH4zV2q7bGLe8WDtV8N3BBxgJo6Ba97/XeRrVttSJns6GaT29MD47j6ehW4n/53xKvXKwzEZgHxlOIZW2YnpAILTuPOuec= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=quicinc.com; spf=pass smtp.mailfrom=quicinc.com; dkim=pass (2048-bit key) header.d=quicinc.com header.i=@quicinc.com header.b=Zt6XL+7J; arc=none smtp.client-ip=205.220.168.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=quicinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=quicinc.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=quicinc.com header.i=@quicinc.com header.b="Zt6XL+7J" Received: from pps.filterd (m0279862.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 517B6UGF016976; Fri, 7 Feb 2025 20:11:58 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= zSgIMbskBs9DQq6cGCzvl2hoxmfK8m643mccdsYsoTU=; b=Zt6XL+7Juok2QSTV JpIjVTmcydmfJoiDMy07HOI2fGyHNbN/ndOW3tQqaOyq+nvWG/cJ+ZIwWH57F64P 1sckyCNzlYCvEPvTuxvyK5epGCR0S+WcKpCaaV5RJ8r470XJvo0wrhLMSbPvWAvK ICSreXYTeJycoPFXkuXi3TPboDc3ZPq5F9kuL/ALz3WyYAiCmNHBHROni1EBNArN INZ30Ee0rtyRuW3TvXiEAXLIN6IYBjTAucm+RedLOk4r5O8llJn2LwMi6oahb0MV YyCnWh/u930JExgmz4y+SqEwYZV3UHwLZN8874x3oZcX1WRpgUW/9Kx3DYA7mGsg ctgHBw== Received: from nalasppmta05.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 44nh0w19mg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 07 Feb 2025 20:11:58 +0000 (GMT) Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA05.qualcomm.com (8.18.1.2/8.18.1.2) with ESMTPS id 517KBvGI022866 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 7 Feb 2025 20:11:57 GMT Received: from [10.110.94.204] (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.9; Fri, 7 Feb 2025 12:11:55 -0800 Message-ID: Date: Fri, 7 Feb 2025 12:11:55 -0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 2/7] drm/msm/hdmi: program HDMI timings during atomic_pre_enable To: Dmitry Baryshkov CC: Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Rob Clark , Sean Paul , Marijn Suijten , Simona Vetter , Simona Vetter , , , , References: <20250124-bridge-hdmi-connector-v6-0-1592632327f7@linaro.org> <20250124-bridge-hdmi-connector-v6-2-1592632327f7@linaro.org> <7fbfc7d5-f6bb-4f99-914a-f91bb7d153fd@quicinc.com> <1b98265e-8766-4504-b374-f7af8203c926@quicinc.com> Content-Language: en-US From: Abhinav Kumar In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: KKL97QBwWp18hsHZe4fhucww8XfK25Sz X-Proofpoint-ORIG-GUID: KKL97QBwWp18hsHZe4fhucww8XfK25Sz X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1057,Hydra:6.0.680,FMLib:17.12.68.34 definitions=2025-02-07_09,2025-02-07_03,2024-11-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 clxscore=1015 bulkscore=0 suspectscore=0 spamscore=0 priorityscore=1501 impostorscore=0 mlxlogscore=999 adultscore=0 malwarescore=0 phishscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2501170000 definitions=main-2502070151 On 2/6/2025 5:19 PM, Dmitry Baryshkov wrote: > On Thu, Feb 06, 2025 at 12:41:30PM -0800, Abhinav Kumar wrote: >> >> >> On 2/3/2025 4:59 PM, Dmitry Baryshkov wrote: >>> On Mon, Feb 03, 2025 at 11:34:00AM -0800, Abhinav Kumar wrote: >>>> >>>> >>>> On 1/24/2025 1:47 PM, Dmitry Baryshkov wrote: >>>>> The mode_set callback is deprecated, it doesn't get the >>>>> drm_bridge_state, just mode-related argumetns. Also Abhinav pointed out >>>>> that HDMI timings should be programmed after setting up HDMI PHY and >>>>> PLL. Rework the code to program HDMI timings at the end of >>>>> atomic_pre_enable(). >>>>> >>>>> Signed-off-by: Dmitry Baryshkov >>>>> --- >>>>> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 23 +++++++++++++++-------- >>>>> 1 file changed, 15 insertions(+), 8 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c >>>>> index d839c71091dcdc3b020fcbba8d698d58ee7fc749..d5ab1f74c0e6f47dc59872c016104e9a84d85e9e 100644 >>>>> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c >>>>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c >>>>> @@ -126,15 +126,26 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi) >>>>> hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val); >>>>> } >>>>> +static void msm_hdmi_bridge_atomic_set_timings(struct hdmi *hdmi, >>>>> + const struct drm_display_mode *mode); >>>>> static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, >>>>> struct drm_bridge_state *old_bridge_state) >>>>> { >>>>> + struct drm_atomic_state *state = old_bridge_state->base.state; >>>>> struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); >>>>> struct hdmi *hdmi = hdmi_bridge->hdmi; >>>>> struct hdmi_phy *phy = hdmi->phy; >>>>> + struct drm_encoder *encoder = bridge->encoder; >>>>> + struct drm_connector *connector; >>>>> + struct drm_connector_state *conn_state; >>>>> + struct drm_crtc_state *crtc_state; >>>>> DBG("power up"); >>>>> + connector = drm_atomic_get_new_connector_for_encoder(state, encoder); >>>>> + conn_state = drm_atomic_get_new_connector_state(state, connector); >>>>> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); >>>>> + >>>>> if (!hdmi->power_on) { >>>>> msm_hdmi_phy_resource_enable(phy); >>>>> msm_hdmi_power_on(bridge); >>>>> @@ -151,6 +162,8 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge, >>>>> if (hdmi->hdcp_ctrl) >>>>> msm_hdmi_hdcp_on(hdmi->hdcp_ctrl); >>>>> + >>>>> + msm_hdmi_bridge_atomic_set_timings(hdmi, &crtc_state->adjusted_mode); >>>>> } >>>> >>>> This addresses my comment about setting up the HDMI timing registers before >>>> setting up the timing engine registers. >>>> >>>> But prior to this change, mode_set was doing the same thing as >>>> msm_hdmi_bridge_atomic_set_timings() which means >>>> msm_hdmi_bridge_atomic_set_timings() should be called at the beginning of >>>> pre_enable()? >>>> >>>> The controller is enabled in msm_hdmi_set_mode(). So this should be done >>>> before that. >>> >>> In [1] you provided the following order: >>> >>> 1) setup HDMI PHY and PLL >>> 2) setup HDMI video path correctly (HDMI timing registers) >>> 3) setup timing generator to match the HDMI video in (2) >>> 4) Enable timing engine >>> >>> This means htat msm_hdmi_bridge_atomic_set_timings() should come at the >>> end of msm_hdmi_bridge_atomic_pre_enable(), not in the beginning / >>> middle of it. >>> >>> [1] https://lore.kernel.org/dri-devel/8dd4a43e-d83c-1f36-21ff-61e13ff751e7@quicinc.com/ >>> >> >> Sequence given is correct and is exactly what is given in the docs. What is >> somewhat not clear in the docs is the location of the enable of the HDMI >> controller. This is not there in the above 4 steps. I am referring to the >> enable bit being programmed in msm_hdmi_set_mode(). Ideally till we enable >> the timing engine, it should be okay but what I wanted to do was to keep the >> msm_hdmi_set_mode() as the last call in this function that way we program >> everything and then enable the controller. >> >> This can be done in either way, move it to the beginning of the function or >> move it right before msm_hdmi_set_mode(). I had suggested beginning because >> thats how it was when things were still in mode_set. > > Well.. following your description it might be better to put it after PHY > init. What do you think? > Are you referring to after msm_hdmi_phy_powerup()? Yes, thats fine too.