Message ID | 1668609040-2549-3-git-send-email-quic_kalyant@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp172974wru; Wed, 16 Nov 2022 06:33:28 -0800 (PST) X-Google-Smtp-Source: AA0mqf6Nl/hYno6Zqjy8Amdoz763eIpVjcpbTxMsHWMWcx2g2NiXmmfdaum43s6+hx9JtJ4N4Kj0 X-Received: by 2002:a05:6a00:1955:b0:562:fcae:5f10 with SMTP id s21-20020a056a00195500b00562fcae5f10mr22937470pfk.28.1668609207792; Wed, 16 Nov 2022 06:33:27 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668609207; cv=none; d=google.com; s=arc-20160816; b=ehWFmq9piXbU9Eq12o7aJn1UNljTYq4b9q2FFdJPvoZ1sH+DrI1APtlI0zJZ+n8KVz 875PqyEZZ4vm7+Veu76l0Zp55E41WgQmrqRgvYEcg2YWE2xBlHPtNDTDvm0XkCwiWPr6 r5NpDjZxBE7cl/L2qnPEDsFna+SLqk3tXag1koVZlVsAxIEFBd6qpVSgt56oM1HF1Yeh f3xDTXMR+C9ohgUjR2pG9pJo0rGUyvt8BXifhCPlOtqQYdlHOHn4j3rx2WAo3zMwqQYB xKMvUoySRBrIzegASENwHq+VcBYajs7VRLCCqv8CQFcS+wvwNC/s2dNhlRKUTHKpH+vp Au+g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:in-reply-to:message-id:date:subject :cc:to:from:dkim-signature; bh=wkJ1Y48hEDj/CRFHK6FUI+sLNPDPj7zrZGorfJ3Cj2w=; b=z0xv0TJp52UxEEqI3D02KzmELnrVyChFAv+Bt3AwT1Uh29Y9U68D5NpNu991G+qefj pFZW3/AGe1Nw8zOT4ZMf3L14KNLcPMn3VBf7qvlD/RV3JAeDEd8Ei9Nzpnwem26cIJYs 6GEyF2muag3RI8LrD1hPdXocqLBewpc2NUdmX+pfgY9hnK228zdObjC2zvYep/pBvhAH U0rcYIQRnE9vNO1CCXzQCzVF9KwVMQtVLC8+lMIrIZjj/CbrmglZwD4QIVoI+sOTkRCB arKnbauD58LplxSAyHKq933+sBUmTD/uI0K/nyfwk9SJuLl0Jw3Z24we1IUTldKR/2LQ QfeA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=mdEun6Ev; 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=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k124-20020a632482000000b0045a43a568c0si14078251pgk.606.2022.11.16.06.33.01; Wed, 16 Nov 2022 06:33:27 -0800 (PST) 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=@quicinc.com header.s=qcppdkim1 header.b=mdEun6Ev; 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=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233109AbiKPObD (ORCPT <rfc822;just.gull.subs@gmail.com> + 99 others); Wed, 16 Nov 2022 09:31:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54462 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232049AbiKPOay (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 16 Nov 2022 09:30:54 -0500 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 72F2F186CD; Wed, 16 Nov 2022 06:30:54 -0800 (PST) Received: from pps.filterd (m0279867.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2AGC02U2029056; Wed, 16 Nov 2022 14:30:50 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : in-reply-to : references; s=qcppdkim1; bh=wkJ1Y48hEDj/CRFHK6FUI+sLNPDPj7zrZGorfJ3Cj2w=; b=mdEun6Ev5qC9r4scOhc7B01yBh2o5KRoCHM4P2Px2jczDuziry+jdrABT06JjwQntLFj n/WffITmZl8jAc9DF3fLsU0JWT5egnleSwshypac11ywpf1n4uLrhiVcDu5ZHIiOfsmL hTzyrfzpOO08Bmb+bJEvHi7BOdilVruS2DlH7l9UH/pMt9Wj8FSqT0R9DyEIUKHXnsAu 6KQ5PFh6JP6n19x6i+nDDwZ9UWukSbkreL9fTuNnpNj90BwBQhwNkP+Te4kWD3/SupuR hAC0jq7/WDcReN+itpvMnLSPupj2mtJtR6M1yqXAXb8zo4eYCecMIaM4g2ld4ABhJq8C /A== Received: from apblrppmta01.qualcomm.com (blr-bdr-fw-01_GlobalNAT_AllZones-Outside.qualcomm.com [103.229.18.19]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3kvyfm8aq5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 16 Nov 2022 14:30:50 +0000 Received: from pps.filterd (APBLRPPMTA01.qualcomm.com [127.0.0.1]) by APBLRPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTP id 2AGETMLw005215; Wed, 16 Nov 2022 14:30:46 GMT Received: from pps.reinject (localhost [127.0.0.1]) by APBLRPPMTA01.qualcomm.com (PPS) with ESMTPS id 3kt4jkxbcm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 16 Nov 2022 14:30:46 +0000 Received: from APBLRPPMTA01.qualcomm.com (APBLRPPMTA01.qualcomm.com [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 2AGEUklM008049; Wed, 16 Nov 2022 14:30:46 GMT Received: from kalyant-linux.qualcomm.com (kalyant-linux.qualcomm.com [10.204.66.210]) by APBLRPPMTA01.qualcomm.com (PPS) with ESMTP id 2AGEUksx008046; Wed, 16 Nov 2022 14:30:46 +0000 Received: by kalyant-linux.qualcomm.com (Postfix, from userid 94428) id 921D148C2; Wed, 16 Nov 2022 06:30:45 -0800 (PST) From: Kalyan Thota <quic_kalyant@quicinc.com> To: dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, devicetree@vger.kernel.org Cc: Kalyan Thota <quic_kalyant@quicinc.com>, linux-kernel@vger.kernel.org, robdclark@chromium.org, dianders@chromium.org, swboyd@chromium.org, quic_vpolimer@quicinc.com, dmitry.baryshkov@linaro.org, quic_abhinavk@quicinc.com Subject: [PATCH v2 2/3] drm/msm/disp/dpu1: add helper to know if display is pluggable Date: Wed, 16 Nov 2022 06:30:39 -0800 Message-Id: <1668609040-2549-3-git-send-email-quic_kalyant@quicinc.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1668609040-2549-1-git-send-email-quic_kalyant@quicinc.com> References: <1668609040-2549-1-git-send-email-quic_kalyant@quicinc.com> X-QCInternal: smtphost X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: mkKminuOSsAhzOcCXC25zH4He9Kt9hWW X-Proofpoint-ORIG-GUID: mkKminuOSsAhzOcCXC25zH4He9Kt9hWW X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-11-16_03,2022-11-16_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 bulkscore=0 lowpriorityscore=0 priorityscore=1501 malwarescore=0 suspectscore=0 adultscore=0 impostorscore=0 mlxlogscore=997 phishscore=0 spamscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2211160100 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE autolearn=no 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: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749663568196061351?= X-GMAIL-MSGID: =?utf-8?q?1749663568196061351?= |
Series |
add color management support for the crtc
|
|
Commit Message
Kalyan Thota
Nov. 16, 2022, 2:30 p.m. UTC
Since DRM encoder type for few encoders can be similar
(like eDP and DP) find out if the interface supports HPD
from encoder bridge to differentiate between builtin
and pluggable displays.
Changes in v1:
- add connector type in the disp_info (Dmitry)
- add helper functions to know encoder type
- update commit text reflecting the change
Changes in v2:
- avoid hardcode of connector type for DSI as it may not be true (Dmitry)
- get the HPD information from encoder bridge
Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++
2 files changed, 22 insertions(+)
Comments
On 16/11/2022 17:30, Kalyan Thota wrote: > Since DRM encoder type for few encoders can be similar > (like eDP and DP) find out if the interface supports HPD > from encoder bridge to differentiate between builtin > and pluggable displays. > > Changes in v1: > - add connector type in the disp_info (Dmitry) > - add helper functions to know encoder type > - update commit text reflecting the change > > Changes in v2: > - avoid hardcode of connector type for DSI as it may not be true (Dmitry) > - get the HPD information from encoder bridge > > Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++ > 2 files changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 9c6817b..be93269 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -15,6 +15,7 @@ > #include <drm/drm_crtc.h> > #include <drm/drm_file.h> > #include <drm/drm_probe_helper.h> > +#include <drm/drm_bridge.h> > > #include "msm_drv.h" > #include "dpu_kms.h" > @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { > 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 > }; > > +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) > +{ > + struct drm_bridge *bridge; > + int ops = 0; > + > + if (!encoder) > + return false; > + > + /* Get last bridge in the chain to determine pluggable state */ > + drm_for_each_bridge_in_chain(encoder, bridge) > + if (!drm_bridge_get_next_bridge(bridge)) > + ops = bridge->ops; > + > + return ops & DRM_BRIDGE_OP_HPD; No. This is not what you should be checking (hint: polled connectors also can be pluggable). Please check the type of the actual connector connected to this encoder. > +} > > bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc) > { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > index 9e7236e..691ab57 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > @@ -224,4 +224,10 @@ void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, > */ > bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); > > +/** > + * dpu_encoder_is_pluggable - find if the encoder is of type pluggable > + * @drm_enc: Pointer to previously created drm encoder structure > + */ > +bool dpu_encoder_is_pluggable(struct drm_encoder *drm_enc); > + > #endif /* __DPU_ENCODER_H__ */
On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: > On 16/11/2022 17:30, Kalyan Thota wrote: >> Since DRM encoder type for few encoders can be similar >> (like eDP and DP) find out if the interface supports HPD >> from encoder bridge to differentiate between builtin >> and pluggable displays. >> >> Changes in v1: >> - add connector type in the disp_info (Dmitry) >> - add helper functions to know encoder type >> - update commit text reflecting the change >> >> Changes in v2: >> - avoid hardcode of connector type for DSI as it may not be true (Dmitry) >> - get the HPD information from encoder bridge >> >> Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++ >> 2 files changed, 22 insertions(+) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index 9c6817b..be93269 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -15,6 +15,7 @@ >> #include <drm/drm_crtc.h> >> #include <drm/drm_file.h> >> #include <drm/drm_probe_helper.h> >> +#include <drm/drm_bridge.h> >> #include "msm_drv.h" >> #include "dpu_kms.h" >> @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { >> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 >> }; >> +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) >> +{ >> + struct drm_bridge *bridge; >> + int ops = 0; >> + >> + if (!encoder) >> + return false; >> + >> + /* Get last bridge in the chain to determine pluggable state */ >> + drm_for_each_bridge_in_chain(encoder, bridge) >> + if (!drm_bridge_get_next_bridge(bridge)) >> + ops = bridge->ops; >> + >> + return ops & DRM_BRIDGE_OP_HPD; > > No. This is not what you should be checking (hint: polled connectors > also can be pluggable). > > Please check the type of the actual connector connected to this encoder. > Even if we check the connector type as DSI or eDP that does not necessarily mean its built-in. We can even use DSI or eDP as a pluggable display. Thats why we thought of this check. >> +} >> bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc) >> { >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> index 9e7236e..691ab57 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> @@ -224,4 +224,10 @@ void dpu_encoder_cleanup_wb_job(struct >> drm_encoder *drm_enc, >> */ >> bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); >> +/** >> + * dpu_encoder_is_pluggable - find if the encoder is of type pluggable >> + * @drm_enc: Pointer to previously created drm encoder structure >> + */ >> +bool dpu_encoder_is_pluggable(struct drm_encoder *drm_enc); >> + >> #endif /* __DPU_ENCODER_H__ */ >
On 16/11/2022 18:11, Abhinav Kumar wrote: > > > On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: >> On 16/11/2022 17:30, Kalyan Thota wrote: >>> Since DRM encoder type for few encoders can be similar >>> (like eDP and DP) find out if the interface supports HPD >>> from encoder bridge to differentiate between builtin >>> and pluggable displays. >>> >>> Changes in v1: >>> - add connector type in the disp_info (Dmitry) >>> - add helper functions to know encoder type >>> - update commit text reflecting the change >>> >>> Changes in v2: >>> - avoid hardcode of connector type for DSI as it may not be true >>> (Dmitry) >>> - get the HPD information from encoder bridge >>> >>> Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++ >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++ >>> 2 files changed, 22 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> index 9c6817b..be93269 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> @@ -15,6 +15,7 @@ >>> #include <drm/drm_crtc.h> >>> #include <drm/drm_file.h> >>> #include <drm/drm_probe_helper.h> >>> +#include <drm/drm_bridge.h> >>> #include "msm_drv.h" >>> #include "dpu_kms.h" >>> @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { >>> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 >>> }; >>> +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) >>> +{ >>> + struct drm_bridge *bridge; >>> + int ops = 0; >>> + >>> + if (!encoder) >>> + return false; >>> + >>> + /* Get last bridge in the chain to determine pluggable state */ >>> + drm_for_each_bridge_in_chain(encoder, bridge) >>> + if (!drm_bridge_get_next_bridge(bridge)) >>> + ops = bridge->ops; >>> + >>> + return ops & DRM_BRIDGE_OP_HPD; >> >> No. This is not what you should be checking (hint: polled connectors >> also can be pluggable). >> >> Please check the type of the actual connector connected to this encoder. >> > > Even if we check the connector type as DSI or eDP that does not > necessarily mean its built-in. > > We can even use DSI or eDP as a pluggable display. Well, I don't think so. eDP and DSI connectors are not pluggable per design. One can use them so, but they are not thought to be used this way. Unlike e.g. HDMI, DP, VGA, etc. I would say LVDS, eDP, DSI, DPI and SPI can be assumed to be constantly plugged. Compare this with Composite, SVIDEO, 9PinDIN, TV. They can be assumed to be external even if they do not have the HPD (or even polling). And these connectors usually don't have it. > > Thats why we thought of this check. >
On 11/16/2022 7:18 AM, Dmitry Baryshkov wrote: > On 16/11/2022 18:11, Abhinav Kumar wrote: >> >> >> On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: >>> On 16/11/2022 17:30, Kalyan Thota wrote: >>>> Since DRM encoder type for few encoders can be similar >>>> (like eDP and DP) find out if the interface supports HPD >>>> from encoder bridge to differentiate between builtin >>>> and pluggable displays. >>>> >>>> Changes in v1: >>>> - add connector type in the disp_info (Dmitry) >>>> - add helper functions to know encoder type >>>> - update commit text reflecting the change >>>> >>>> Changes in v2: >>>> - avoid hardcode of connector type for DSI as it may not be true >>>> (Dmitry) >>>> - get the HPD information from encoder bridge >>>> >>>> Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> >>>> --- >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++ >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++ >>>> 2 files changed, 22 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>> index 9c6817b..be93269 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>> @@ -15,6 +15,7 @@ >>>> #include <drm/drm_crtc.h> >>>> #include <drm/drm_file.h> >>>> #include <drm/drm_probe_helper.h> >>>> +#include <drm/drm_bridge.h> >>>> #include "msm_drv.h" >>>> #include "dpu_kms.h" >>>> @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { >>>> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 >>>> }; >>>> +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) >>>> +{ >>>> + struct drm_bridge *bridge; >>>> + int ops = 0; >>>> + >>>> + if (!encoder) >>>> + return false; >>>> + >>>> + /* Get last bridge in the chain to determine pluggable state */ >>>> + drm_for_each_bridge_in_chain(encoder, bridge) >>>> + if (!drm_bridge_get_next_bridge(bridge)) >>>> + ops = bridge->ops; >>>> + >>>> + return ops & DRM_BRIDGE_OP_HPD; >>> >>> No. This is not what you should be checking (hint: polled connectors >>> also can be pluggable). >>> >>> Please check the type of the actual connector connected to this encoder. >>> >> >> Even if we check the connector type as DSI or eDP that does not >> necessarily mean its built-in. >> >> We can even use DSI or eDP as a pluggable display. > > Well, I don't think so. eDP and DSI connectors are not pluggable per > design. One can use them so, but they are not thought to be used this > way. Unlike e.g. HDMI, DP, VGA, etc. > We have had many products where we used HDMI as the primary display where the HPD line was disconnected in the design, so now if we generalize all HDMI connectors to be pluggable we can never enable color management on those even though DSI is not even used in that product. Thats why I felt we should rely on the HPD_OPS as that way we know that it will be set only if HPD will be used. Wouldnt it be just better to also check polling displays to complete this check? Is there a way to do it? > I would say LVDS, eDP, DSI, DPI and SPI can be assumed to be constantly > plugged. > > Compare this with Composite, SVIDEO, 9PinDIN, TV. They can be assumed to > be external even if they do not have the HPD (or even polling). And > these connectors usually don't have it. > >> >> Thats why we thought of this check. >>
On 16/11/2022 18:35, Abhinav Kumar wrote: > > > On 11/16/2022 7:18 AM, Dmitry Baryshkov wrote: >> On 16/11/2022 18:11, Abhinav Kumar wrote: >>> >>> >>> On 11/16/2022 7:08 AM, Dmitry Baryshkov wrote: >>>> On 16/11/2022 17:30, Kalyan Thota wrote: >>>>> Since DRM encoder type for few encoders can be similar >>>>> (like eDP and DP) find out if the interface supports HPD >>>>> from encoder bridge to differentiate between builtin >>>>> and pluggable displays. >>>>> >>>>> Changes in v1: >>>>> - add connector type in the disp_info (Dmitry) >>>>> - add helper functions to know encoder type >>>>> - update commit text reflecting the change >>>>> >>>>> Changes in v2: >>>>> - avoid hardcode of connector type for DSI as it may not be true >>>>> (Dmitry) >>>>> - get the HPD information from encoder bridge >>>>> >>>>> Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> >>>>> --- >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++ >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++ >>>>> 2 files changed, 22 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>> index 9c6817b..be93269 100644 >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>> @@ -15,6 +15,7 @@ >>>>> #include <drm/drm_crtc.h> >>>>> #include <drm/drm_file.h> >>>>> #include <drm/drm_probe_helper.h> >>>>> +#include <drm/drm_bridge.h> >>>>> #include "msm_drv.h" >>>>> #include "dpu_kms.h" >>>>> @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { >>>>> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 >>>>> }; >>>>> +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) >>>>> +{ >>>>> + struct drm_bridge *bridge; >>>>> + int ops = 0; >>>>> + >>>>> + if (!encoder) >>>>> + return false; >>>>> + >>>>> + /* Get last bridge in the chain to determine pluggable state */ >>>>> + drm_for_each_bridge_in_chain(encoder, bridge) >>>>> + if (!drm_bridge_get_next_bridge(bridge)) >>>>> + ops = bridge->ops; >>>>> + >>>>> + return ops & DRM_BRIDGE_OP_HPD; >>>> >>>> No. This is not what you should be checking (hint: polled connectors >>>> also can be pluggable). >>>> >>>> Please check the type of the actual connector connected to this >>>> encoder. >>>> >>> >>> Even if we check the connector type as DSI or eDP that does not >>> necessarily mean its built-in. >>> >>> We can even use DSI or eDP as a pluggable display. >> >> Well, I don't think so. eDP and DSI connectors are not pluggable per >> design. One can use them so, but they are not thought to be used this >> way. Unlike e.g. HDMI, DP, VGA, etc. >> > > We have had many products where we used HDMI as the primary display > where the HPD line was disconnected in the design, so now if we > generalize all HDMI connectors to be pluggable we can never enable color > management on those even though DSI is not even used in that product. Did they use HDMI connector internally? Or was it just a panel wired somehow to the HDMI pins? If the former is true, then they still are pluggable. Even if you don't have a way to detect that via the HPD pin. If the later is the case, then it shouldn't be DRM_MODE_CONNECTOR_HDMI. Well, even if this is an internal HDMI, I'd still use some other connector (e.g. DRM_MODE_CONNECTOR_Unknown) just to point out that this is not an externally visible HDMI connector one assumes to be able to find on the body of the device. Last, but not least, how would you remove DRM_BRIDGE_OPS_HPD from the corresponding bridge driver? > Thats why I felt we should rely on the HPD_OPS as that way we know that > it will be set only if HPD will be used. > > Wouldnt it be just better to also check polling displays to complete > this check? Is there a way to do it? Yes, check DRM_BRIDGE_OP_DETECT. But as I noted, there is a list of connectors that will not ever have HPD or polling, but still always are external. Well, even for VGA there is no good way to detect whether it is plugged in or not (see the comment in display-connector.c). >> I would say LVDS, eDP, DSI, DPI and SPI can be assumed to be >> constantly plugged. >> >> Compare this with Composite, SVIDEO, 9PinDIN, TV. They can be assumed >> to be external even if they do not have the HPD (or even polling). And >> these connectors usually don't have it. >> >>> >>> Thats why we thought of this check. >>>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b..be93269 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -15,6 +15,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_file.h> #include <drm/drm_probe_helper.h> +#include <drm/drm_bridge.h> #include "msm_drv.h" #include "dpu_kms.h" @@ -217,6 +218,21 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_pluggable(struct drm_encoder *encoder) +{ + struct drm_bridge *bridge; + int ops = 0; + + if (!encoder) + return false; + + /* Get last bridge in the chain to determine pluggable state */ + drm_for_each_bridge_in_chain(encoder, bridge) + if (!drm_bridge_get_next_bridge(bridge)) + ops = bridge->ops; + + return ops & DRM_BRIDGE_OP_HPD; +} bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 9e7236e..691ab57 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -224,4 +224,10 @@ void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, */ bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); +/** + * dpu_encoder_is_pluggable - find if the encoder is of type pluggable + * @drm_enc: Pointer to previously created drm encoder structure + */ +bool dpu_encoder_is_pluggable(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */