Re: [PATCH v2 2/3] Subject: [PATCH] drm/mediatek/dp: Add HDCP2.x feature for DisplayPort
Re: [PATCH v2 2/3] Subject: [PATCH] drm/mediatek/dp: Add HDCP2.x feature for DisplayPort
Re: [PATCH] drm/ttm: Fix an invalid freeing on already freed page in error path
On Wed, 2024-02-21 at 11:26 +0100, Christian König wrote: > Am 21.02.24 um 08:33 schrieb Thomas Hellström: > > If caching mode change fails due to, for example, OOM we > > free the allocated pages in a two-step process. First the pages > > for which the caching change has already succeeded. Secondly > > the pages for which a caching change did not succeed. > > > > However the second step was incorrectly freeing the pages already > > freed in the first step. > > > > Fix. > > > > Signed-off-by: Thomas Hellström > > Fixes: 379989e7cbdc ("drm/ttm/pool: Fix ttm_pool_alloc error path") > > Cc: Christian König > > Cc: Dave Airlie > > Cc: Christian Koenig > > Cc: Huang Rui > > Cc: dri-devel@lists.freedesktop.org > > Cc: # v6.4+ > > You don't know how much time I've spend staring at this line to find > the > bug in it and haven't seen it. Got bug reports about that for month > as well. Yeah, sorry about that. We should probably have Kunit tests exercising OOM in the pool code involving WC pages. I'll push this to drm-misc-next. /Thomas > > Reviewed-by: Christian König > > > --- > > drivers/gpu/drm/ttm/ttm_pool.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c > > b/drivers/gpu/drm/ttm/ttm_pool.c > > index b62f420a9f96..112438d965ff 100644 > > --- a/drivers/gpu/drm/ttm/ttm_pool.c > > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > > @@ -387,7 +387,7 @@ static void ttm_pool_free_range(struct ttm_pool > > *pool, struct ttm_tt *tt, > > enum ttm_caching caching, > > pgoff_t start_page, pgoff_t > > end_page) > > { > > - struct page **pages = tt->pages; > > + struct page **pages = >pages[start_page]; > > unsigned int order; > > pgoff_t i, nr; > > >
Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg
On Thu, 22 Feb 2024 at 04:10, 'Justin Stitt' via KUnit Development wrote: > > Hi, > > On Wed, Feb 21, 2024 at 05:27:15PM +0800, David Gow wrote: > > The correct format specifier for p - n (both p and n are pointers) is > > %td, as the type should be ptrdiff_t. > > I think %tu is better. d specifies a signed type. I don't doubt that the > warning is fixed but I think %tu represents the type semantics here. > While I agree that this should never be negative, I'd still lean on this being a signed type, for two reasons: - I think, if there's a bug in this code, it's easier to debug this if a 'negative' value were to appear as such. - While, as I understand it, the C spec does provide for a ptrdiff_t-sized unsigned printf specifier in '%tu', the difference between two pointers is always signed: "When two pointers are subtracted, both shall point to elements of the same array object, or one past the last element of the array object; the result is the difference of the subscripts of the two array elements. The size of the result is implementation-defined, and its type (a signed integer type) is ptrdiff_t defined in the header" (Technically, the kernel's ptrdiff_t type isn't defined in stddef.h, so a bit of deviation from the spec is happening anyway, though.) If there's a particularly good reason to make this unsigned in this case, I'd be happy to change it, of course. But I'd otherwise prefer to keep it as-is. Cheers, -- David smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH 8/9] drm/xe/tests: Fix printf format specifiers in xe_migrate test
On Wed, 21 Feb 2024 at 21:05, Lucas De Marchi wrote: > > this has a potential to cause conflicts with upcoming work, so I think > it's better to apply this through drm-xe-next. Let me know if you agree. I disagree. Violently. For this to be fixed, we need to have the printf format checking enabled. And we can't enable it until all the problems have been fixed. Which means that we should *not* have to wait for [N] different trees to fix their issues separately. This should get fixed in the Kunit tree, so that the Kunit tree can just send a pull request to me to enable format checking for the KUnit tests, together with all the fixes. Trying to spread those fixes out to different git branches will only result in pain and pointless dependencies between different trees. Honestly, the reason I noticed the problem in the first place was that the drm tree had a separate bug, that had been apparently noted in linux-next, and *despite* that it made it into a pull request to me and caused new build failures in rc5. So as things are, I am not IN THE LEAST interested in some kind of "let us fix this in the drm tree separately" garbage. We're not making things worse by trying to fix this in different trees. We're fixing this in the Kunit tree, and I do not want to get *more* problems from the drm side. I've had enough. Linus
Re: [PATCH 8/9] drm/xe/tests: Fix printf format specifiers in xe_migrate test
On Wed, Feb 21, 2024 at 05:27:21PM +0800, David Gow wrote: KUNIT_FAIL() is used to fail the xe_migrate test when an error occurs. However, there's a mismatch in the format specifier: '%li' is used to log 'err', which is an 'int'. Use '%i' instead of '%li', and for the case where we're printing an error pointer, just use '%pe', instead of extracting the error code manually with PTR_ERR(). (This also results in a nicer output when the error code is known.) Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs") Signed-off-by: David Gow Reviewed-by: Lucas De Marchi this has a potential to cause conflicts with upcoming work, so I think it's better to apply this through drm-xe-next. Let me know if you agree. thanks Lucas De Marchi --- drivers/gpu/drm/xe/tests/xe_migrate.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_migrate.c b/drivers/gpu/drm/xe/tests/xe_migrate.c index a6523df0f1d3..c347e2c29f81 100644 --- a/drivers/gpu/drm/xe/tests/xe_migrate.c +++ b/drivers/gpu/drm/xe/tests/xe_migrate.c @@ -114,21 +114,21 @@ static void test_copy(struct xe_migrate *m, struct xe_bo *bo, region | XE_BO_NEEDS_CPU_ACCESS); if (IS_ERR(remote)) { - KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %li\n", - str, PTR_ERR(remote)); + KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %pe\n", + str, remote); return; } err = xe_bo_validate(remote, NULL, false); if (err) { - KUNIT_FAIL(test, "Failed to validate system bo for %s: %li\n", + KUNIT_FAIL(test, "Failed to validate system bo for %s: %i\n", str, err); goto out_unlock; } err = xe_bo_vmap(remote); if (err) { - KUNIT_FAIL(test, "Failed to vmap system bo for %s: %li\n", + KUNIT_FAIL(test, "Failed to vmap system bo for %s: %i\n", str, err); goto out_unlock; } -- 2.44.0.rc0.258.g7320e95886-goog
Re: [PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280
On Thu, 22 Feb 2024 at 05:47, Bjorn Andersson wrote: > > On Thu, Feb 22, 2024 at 01:38:45AM +0200, Dmitry Baryshkov wrote: > > On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson > > wrote: > > > > > > When upstreamed the SC7280 DP controllers where described as one being > > > DP and one eDP, but they can infact both be DP or eDP. > > > > > > Extend the list of DP controllers to cover both instances, and rely on > > > the newly introduced mechanism for selecting which mode they should > > > operate in. > > > > > > Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree > > > will continue to select eDP. > > > > > > Signed-off-by: Bjorn Andersson > > > --- > > > drivers/gpu/drm/msm/dp/dp_display.c | 9 +++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > > b/drivers/gpu/drm/msm/dp/dp_display.c > > > index 7b8c695d521a..1792ba9f7259 100644 > > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > > @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = { > > > }; > > > > > > static const struct msm_dp_desc sc7280_dp_descs[] = { > > > - { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, > > > .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true }, > > > + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en > > > = true }, > > > + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en > > > = true }, > > > > I think we need to keep .connector_type here, don't we? > > > > No, Abel removes the need for that in his patches - and while that logic > is slightly broken in the RFC I think it looks good. Let's see v2 first. > > Regards, > Bjorn > > > > + {} > > > +}; > > > + > > > +static const struct msm_dp_desc sc7280_edp_descs[] = { > > > { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, > > > .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true }, > > > {} > > > }; > > > @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = > > > { > > > static const struct of_device_id dp_dt_match[] = { > > > { .compatible = "qcom,sc7180-dp", .data = _dp_descs }, > > > { .compatible = "qcom,sc7280-dp", .data = _dp_descs }, > > > - { .compatible = "qcom,sc7280-edp", .data = _dp_descs }, > > > + { .compatible = "qcom,sc7280-edp", .data = _edp_descs }, > > > { .compatible = "qcom,sc8180x-dp", .data = _dp_descs }, > > > { .compatible = "qcom,sc8180x-edp", .data = _dp_descs }, > > > { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs }, > > > > > > -- > > > 2.25.1 > > > > > > > > > -- > > With best wishes > > Dmitry -- With best wishes Dmitry
Re: [PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching
On Thu, Feb 22, 2024 at 01:50:12AM +0200, Dmitry Baryshkov wrote: > On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson > wrote: > > > > With the ADSP remoteproc loaded pmic_glink can be introduced and wired > > up to provide role and orientation switching signals. > > > > Signed-off-by: Bjorn Andersson > > --- > > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48 > > +++- > > 1 file changed, 47 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > > b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > > index ab498494caea..079bf43b14cc 100644 > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > > @@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 { > > }; > > }; > > > > + pmic-glink { > > + compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink"; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + connector@0 { > > + compatible = "usb-c-connector"; > > + reg = <0>; > > + power-role = "dual"; > > + data-role = "dual"; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + > > + pmic_glink_hs_in: endpoint { > > + remote-endpoint = > > <_1_dwc3_hs>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + > > + pmic_glink_ss_in: endpoint { > > + remote-endpoint = > > <_1_dwc3_ss>; > > This should be connected to the QMP PHY rather than to the USB host. > Ahh, you're right, otherwise the orientation-switch below isn't of much use. > Also it might be better to squash this patch with the patch 8. Or at > least to get redriver into the picture in this patch (and keep only > display-related parts in that patch). > The idea was to only bring in the pmic-glink here and then do the plumbing between all the components separately, but I guess the orientation-switch in the redriver means that it should go here as well... I'll shuffle this into something that makes sense. Thanks, Bjorn > > > + }; > > + }; > > + }; > > + }; > > + }; > > + > > vph_pwr: vph-pwr-regulator { > > compatible = "regulator-fixed"; > > regulator-name = "vph_pwr"; > > @@ -476,7 +511,16 @@ _1 { > > }; > > > > _1_dwc3 { > > - dr_mode = "peripheral"; > > + dr_mode = "otg"; > > + usb-role-switch; > > +}; > > + > > +_1_dwc3_hs { > > + remote-endpoint = <_glink_hs_in>; > > +}; > > + > > +_1_dwc3_ss { > > + remote-endpoint = <_glink_ss_in>; > > }; > > > > _1_hsphy { > > @@ -491,6 +535,8 @@ _1_qmpphy { > > vdda-phy-supply = <_l6b_1p2>; > > vdda-pll-supply = <_l1b_0p912>; > > > > + orientation-switch; > > + > > status = "okay"; > > }; > > > > > > -- > > 2.25.1 > > > > > -- > With best wishes > Dmitry
Re: [PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280
On Thu, Feb 22, 2024 at 01:38:45AM +0200, Dmitry Baryshkov wrote: > On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson > wrote: > > > > When upstreamed the SC7280 DP controllers where described as one being > > DP and one eDP, but they can infact both be DP or eDP. > > > > Extend the list of DP controllers to cover both instances, and rely on > > the newly introduced mechanism for selecting which mode they should > > operate in. > > > > Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree > > will continue to select eDP. > > > > Signed-off-by: Bjorn Andersson > > --- > > drivers/gpu/drm/msm/dp/dp_display.c | 9 +++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > b/drivers/gpu/drm/msm/dp/dp_display.c > > index 7b8c695d521a..1792ba9f7259 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = { > > }; > > > > static const struct msm_dp_desc sc7280_dp_descs[] = { > > - { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, > > .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true }, > > + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = > > true }, > > + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = > > true }, > > I think we need to keep .connector_type here, don't we? > No, Abel removes the need for that in his patches - and while that logic is slightly broken in the RFC I think it looks good. Regards, Bjorn > > + {} > > +}; > > + > > +static const struct msm_dp_desc sc7280_edp_descs[] = { > > { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, > > .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true }, > > {} > > }; > > @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = { > > static const struct of_device_id dp_dt_match[] = { > > { .compatible = "qcom,sc7180-dp", .data = _dp_descs }, > > { .compatible = "qcom,sc7280-dp", .data = _dp_descs }, > > - { .compatible = "qcom,sc7280-edp", .data = _dp_descs }, > > + { .compatible = "qcom,sc7280-edp", .data = _edp_descs }, > > { .compatible = "qcom,sc8180x-dp", .data = _dp_descs }, > > { .compatible = "qcom,sc8180x-edp", .data = _dp_descs }, > > { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs }, > > > > -- > > 2.25.1 > > > > > -- > With best wishes > Dmitry
Re: [PATCH RFC 3/3] drm/msm/dp: Add support for the X1E80100
On Wed, Feb 21, 2024 at 12:50:33AM +0200, Abel Vesa wrote: > Add the X1E80100 DP descs and compatible. This platform will be using > a single compatible for both eDP and DP mode. The actual mode will > be set in devicetree via is-edp flag. > > Signed-off-by: Abel Vesa > --- > drivers/gpu/drm/msm/dp/dp_display.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index 9e58285d4ec6..7b8c695d521a 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -171,6 +171,14 @@ static const struct msm_dp_desc sm8650_dp_descs[] = { > {} > }; > > +static const struct msm_dp_desc x1e80100_dp_descs[] = { > + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = > true }, > + { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = > true }, > + { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .wide_bus_en = > true }, > + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .wide_bus_en = > true }, > + {} > +}; > + > static const struct of_device_id dp_dt_match[] = { > { .compatible = "qcom,sc7180-dp", .data = _dp_descs }, > { .compatible = "qcom,sc7280-dp", .data = _dp_descs }, > @@ -179,6 +187,7 @@ static const struct of_device_id dp_dt_match[] = { > { .compatible = "qcom,sc8180x-edp", .data = _dp_descs }, > { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs }, > { .compatible = "qcom,sc8280xp-edp", .data = _edp_descs }, > + { .compatible = "qcom,x1e80100-dp", .data = _dp_descs }, This doesn't look like alphabetical order. Regards, Bjorn > { .compatible = "qcom,sdm845-dp", .data = _dp_descs }, > { .compatible = "qcom,sm8350-dp", .data = _dp_descs }, > { .compatible = "qcom,sm8650-dp", .data = _dp_descs }, > > -- > 2.34.1 >
Re: [PATCH RFC 1/3] dt-bindings: display: msm: dp-controller: document X1E80100 compatible
On Wed, Feb 21, 2024 at 12:50:31AM +0200, Abel Vesa wrote: > Add the X1E80100 to the list of compatibles and docoment the is-edp s/docoment/document/ > flag. This new flag will be used from now on to dictate the mode from s/from now on// Perhaps cleaner to spell out that the controllers are expected to operate in DP mode by default, and this flag can be used to select eDP mode. > devicetree, instead of having separate compatibles for eDP and DP. > Regards, Bjorn > Signed-off-by: Abel Vesa > --- > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > index ae53cbfb2193..ed11852e403d 100644 > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > @@ -27,6 +27,7 @@ properties: >- qcom,sdm845-dp >- qcom,sm8350-dp >- qcom,sm8650-dp > + - qcom,x1e80100-dp >- items: >- enum: >- qcom,sm8150-dp > @@ -73,6 +74,11 @@ properties: >- description: phy 0 parent >- description: phy 1 parent > > + is-edp: > +$ref: /schemas/types.yaml#/definitions/flag > +description: > + Tells the controller to switch to eDP mode > + >phys: > maxItems: 1 > > > -- > 2.34.1 >
Re: [PATCH RFC 2/3] drm/msm/dp: Add support for setting the eDP mode from devicetree
On Wed, Feb 21, 2024 at 12:50:32AM +0200, Abel Vesa wrote: > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h > b/drivers/gpu/drm/msm/dp/dp_ctrl.h > index fa014cee7e21..a10d1b19d172 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h > @@ -32,6 +32,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct > dp_link *link, > struct phy *phy); > > void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable); > +int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int mode); > void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl); > void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl); > void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl); > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index e4433891becb..9e58285d4ec6 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -1229,6 +1229,7 @@ static int dp_display_probe(struct platform_device > *pdev) > int rc = 0; > struct dp_display_private *dp; > const struct msm_dp_desc *desc; > + bool is_edp = false; > > if (!pdev || !pdev->dev.of_node) { > DRM_ERROR("pdev not found\n"); > @@ -1243,13 +1244,19 @@ static int dp_display_probe(struct platform_device > *pdev) > if (!desc) > return -EINVAL; > > + if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP || dp is just allocated, and will be 0. You need to check desc->connector_type here. Regards, Bjorn > + of_property_read_bool(pdev->dev.of_node, "is-edp")) > + is_edp = true; > + > + dp->dp_display.is_edp = is_edp; > dp->dp_display.pdev = pdev; > dp->name = "drm_dp"; > dp->id = desc->id; > - dp->dp_display.connector_type = desc->connector_type; > dp->wide_bus_en = desc->wide_bus_en; > - dp->dp_display.is_edp = > - (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP); > + > + dp->dp_display.connector_type = is_edp ? > + DRM_MODE_CONNECTOR_eDP : > + DRM_MODE_CONNECTOR_DisplayPort; > > rc = dp_init_sub_modules(dp); > if (rc) { > @@ -1257,6 +1264,12 @@ static int dp_display_probe(struct platform_device > *pdev) > return -EPROBE_DEFER; > } > > + rc = dp_ctrl_phy_set_mode(dp->ctrl, is_edp ? PHY_SUBMODE_EDP : > PHY_SUBMODE_DP); > + if (rc) { > + DRM_ERROR("setting PHY submode failed\n"); > + goto err; > + } > + > /* setup event q */ > mutex_init(>event_mutex); > init_waitqueue_head(>event_q); > > -- > 2.34.1 >
Re: [PATCH] drm: ci: uprev IGT
On Tue, 20 Feb 2024 at 16:31, Helen Koike wrote: > > > > On 20/02/2024 09:17, Dmitry Baryshkov wrote: > > Bump IGT revision to pick up Rob Clark's fixes for the msm driver: > > > > - msm_submit@invalid-duplicate-bo-submit,Fail > > > > Signed-off-by: Dmitry Baryshkov > > Do you have a gitlab pipeline link I can check? Before uprev: https://gitlab.freedesktop.org/drm/msm/-/pipelines/1109455 After uprev: https://gitlab.freedesktop.org/drm/msm/-/pipelines/1109501 -- With best wishes Dmitry
Re: [PATCH 6/6] phy: qcom-qmp-combo: fix type-c switch registration
On Sat, Feb 17, 2024 at 04:02:28PM +0100, Johan Hovold wrote: > Due to a long-standing issue in driver core, drivers may not probe defer > after having registered child devices to avoid triggering a probe > deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of > -EPROBE_DEFER")). > > Move registration of the typec switch to after looking up clocks and > other resources. > > Note that PHY creation can in theory also trigger a probe deferral when > a 'phy' supply is used. This does not seem to affect the QMP PHY driver > but the PHY subsystem should be reworked to address this (i.e. by > separating initialisation and registration of the PHY). > > Fixes: 2851117f8f42 ("phy: qcom-qmp-combo: Introduce orientation switching") > Cc: sta...@vger.kernel.org # 6.5 > Cc: Bjorn Andersson > Signed-off-by: Johan Hovold Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index e19d6a084f10..17c4ad7553a5 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -3562,10 +3562,6 @@ static int qmp_combo_probe(struct platform_device > *pdev) > if (ret) > return ret; > > - ret = qmp_combo_typec_switch_register(qmp); > - if (ret) > - return ret; > - > /* Check for legacy binding with child nodes. */ > usb_np = of_get_child_by_name(dev->of_node, "usb3-phy"); > if (usb_np) { > @@ -3585,6 +3581,10 @@ static int qmp_combo_probe(struct platform_device > *pdev) > if (ret) > goto err_node_put; > > + ret = qmp_combo_typec_switch_register(qmp); > + if (ret) > + goto err_node_put; > + > ret = drm_aux_bridge_register(dev); > if (ret) > goto err_node_put; > -- > 2.43.0 >
Re: [PATCH 5/6] phy: qcom-qmp-combo: fix drm bridge registration
On Sat, Feb 17, 2024 at 04:02:27PM +0100, Johan Hovold wrote: > Due to a long-standing issue in driver core, drivers may not probe defer > after having registered child devices to avoid triggering a probe > deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of > -EPROBE_DEFER")). > > This could potentially also trigger a bug in the DRM bridge > implementation which does not expect bridges to go away even if device > links may avoid triggering this (when enabled). > > Move registration of the DRM aux bridge to after looking up clocks and > other resources. > > Note that PHY creation can in theory also trigger a probe deferral when > a 'phy' supply is used. This does not seem to affect the QMP PHY driver > but the PHY subsystem should be reworked to address this (i.e. by > separating initialisation and registration of the PHY). > > Fixes: 35921910bbd0 ("phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE") > Fixes: 1904c3f578dc ("phy: qcom-qmp-combo: Introduce drm_bridge") > Cc: sta...@vger.kernel.org # 6.5 > Cc: Bjorn Andersson > Cc: Dmitry Baryshkov > Signed-off-by: Johan Hovold Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index 1ad10110dd25..e19d6a084f10 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -3566,10 +3566,6 @@ static int qmp_combo_probe(struct platform_device > *pdev) > if (ret) > return ret; > > - ret = drm_aux_bridge_register(dev); > - if (ret) > - return ret; > - > /* Check for legacy binding with child nodes. */ > usb_np = of_get_child_by_name(dev->of_node, "usb3-phy"); > if (usb_np) { > @@ -3589,6 +3585,10 @@ static int qmp_combo_probe(struct platform_device > *pdev) > if (ret) > goto err_node_put; > > + ret = drm_aux_bridge_register(dev); > + if (ret) > + goto err_node_put; > + > pm_runtime_set_active(dev); > ret = devm_pm_runtime_enable(dev); > if (ret) > -- > 2.43.0 >
Re: [PATCH 4/6] soc: qcom: pmic_glink: Fix boot when QRTR=m
On Sat, Feb 17, 2024 at 04:02:26PM +0100, Johan Hovold wrote: > From: Rob Clark > > We need to bail out before adding/removing devices if we are going to > -EPROBE_DEFER. Otherwise boot can get stuck in a probe deferral loop due > to a long-standing issue in driver core (see fbc35b45f9f6 ("Add > documentation on meaning of -EPROBE_DEFER")). > > Deregistering the altmode child device can potentially also trigger bugs > in the DRM bridge implementation, which does not expect bridges to go > away. > > Suggested-by: Dmitry Baryshkov > Signed-off-by: Rob Clark > Link: https://lore.kernel.org/r/20231213210644.8702-1-robdcl...@gmail.com > [ johan: rebase on 6.8-rc4, amend commit message and mention DRM ] > Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK > driver") > Cc: sta...@vger.kernel.org # 6.3 > Cc: Bjorn Andersson > Signed-off-by: Johan Hovold Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/soc/qcom/pmic_glink.c | 21 +++-- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c > index f4bfd24386f1..f913e9bd57ed 100644 > --- a/drivers/soc/qcom/pmic_glink.c > +++ b/drivers/soc/qcom/pmic_glink.c > @@ -265,10 +265,17 @@ static int pmic_glink_probe(struct platform_device > *pdev) > > pg->client_mask = *match_data; > > + pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg); > + if (IS_ERR(pg->pdr)) { > + ret = dev_err_probe(>dev, PTR_ERR(pg->pdr), > + "failed to initialize pdr\n"); > + return ret; > + } > + > if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI)) { > ret = pmic_glink_add_aux_device(pg, >ucsi_aux, "ucsi"); > if (ret) > - return ret; > + goto out_release_pdr_handle; > } > if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE)) { > ret = pmic_glink_add_aux_device(pg, >altmode_aux, > "altmode"); > @@ -281,17 +288,11 @@ static int pmic_glink_probe(struct platform_device > *pdev) > goto out_release_altmode_aux; > } > > - pg->pdr = pdr_handle_alloc(pmic_glink_pdr_callback, pg); > - if (IS_ERR(pg->pdr)) { > - ret = dev_err_probe(>dev, PTR_ERR(pg->pdr), "failed to > initialize pdr\n"); > - goto out_release_aux_devices; > - } > - > service = pdr_add_lookup(pg->pdr, "tms/servreg", "msm/adsp/charger_pd"); > if (IS_ERR(service)) { > ret = dev_err_probe(>dev, PTR_ERR(service), > "failed adding pdr lookup for > charger_pd\n"); > - goto out_release_pdr_handle; > + goto out_release_aux_devices; > } > > mutex_lock(&__pmic_glink_lock); > @@ -300,8 +301,6 @@ static int pmic_glink_probe(struct platform_device *pdev) > > return 0; > > -out_release_pdr_handle: > - pdr_handle_release(pg->pdr); > out_release_aux_devices: > if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT)) > pmic_glink_del_aux_device(pg, >ps_aux); > @@ -311,6 +310,8 @@ static int pmic_glink_probe(struct platform_device *pdev) > out_release_ucsi_aux: > if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI)) > pmic_glink_del_aux_device(pg, >ucsi_aux); > +out_release_pdr_handle: > + pdr_handle_release(pg->pdr); > > return ret; > } > -- > 2.43.0 >
Re: [PATCH 3/6] soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free
On Sat, Feb 17, 2024 at 04:02:25PM +0100, Johan Hovold wrote: > A recent DRM series purporting to simplify support for "transparent > bridges" and handling of probe deferrals ironically exposed a > use-after-free issue on pmic_glink_altmode probe deferral. > > This has manifested itself as the display subsystem occasionally failing > to initialise and NULL-pointer dereferences during boot of machines like > the Lenovo ThinkPad X13s. > > Specifically, the dp-hpd bridge is currently registered before all > resources have been acquired which means that it can also be > deregistered on probe deferrals. > > In the meantime there is a race window where the new aux bridge driver > (or PHY driver previously) may have looked up the dp-hpd bridge and > stored a (non-reference-counted) pointer to the bridge which is about to > be deallocated. > > When the display controller is later initialised, this triggers a > use-after-free when attaching the bridges: > > dp -> aux -> dp-hpd (freed) > > which may, for example, result in the freed bridge failing to attach: > > [drm:drm_bridge_attach [drm]] *ERROR* failed to attach bridge > /soc@0/phy@88eb000 to encoder TMDS-31: -16 > > or a NULL-pointer dereference: > > Unable to handle kernel NULL pointer dereference at virtual address > > ... > Call trace: > drm_bridge_attach+0x70/0x1a8 [drm] > drm_aux_bridge_attach+0x24/0x38 [aux_bridge] > drm_bridge_attach+0x80/0x1a8 [drm] > dp_bridge_init+0xa8/0x15c [msm] > msm_dp_modeset_init+0x28/0xc4 [msm] > > The DRM bridge implementation is clearly fragile and implicitly built on > the assumption that bridges may never go away. In this case, the fix is > to move the bridge registration in the pmic_glink_altmode driver to > after all resources have been looked up. > > Incidentally, with the new dp-hpd bridge implementation, which registers > child devices, this is also a requirement due to a long-standing issue > in driver core that can otherwise lead to a probe deferral loop (see > fbc35b45f9f6 ("Add documentation on meaning of -EPROBE_DEFER")). > > Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support") > Fixes: 2bcca96abfbf ("soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE") > Cc: sta...@vger.kernel.org # 6.3 > Cc: Bjorn Andersson > Cc: Dmitry Baryshkov > Signed-off-by: Johan Hovold Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/soc/qcom/pmic_glink_altmode.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/soc/qcom/pmic_glink_altmode.c > b/drivers/soc/qcom/pmic_glink_altmode.c > index 5fcd0fdd2faa..b3808fc24c69 100644 > --- a/drivers/soc/qcom/pmic_glink_altmode.c > +++ b/drivers/soc/qcom/pmic_glink_altmode.c > @@ -76,7 +76,7 @@ struct pmic_glink_altmode_port { > > struct work_struct work; > > - struct device *bridge; > + struct auxiliary_device *bridge; > > enum typec_orientation orientation; > u16 svid; > @@ -230,7 +230,7 @@ static void pmic_glink_altmode_worker(struct work_struct > *work) > else > pmic_glink_altmode_enable_usb(altmode, alt_port); > > - drm_aux_hpd_bridge_notify(alt_port->bridge, > + drm_aux_hpd_bridge_notify(_port->bridge->dev, > alt_port->hpd_state ? > connector_status_connected : > connector_status_disconnected); > @@ -454,7 +454,7 @@ static int pmic_glink_altmode_probe(struct > auxiliary_device *adev, > alt_port->index = port; > INIT_WORK(_port->work, pmic_glink_altmode_worker); > > - alt_port->bridge = drm_dp_hpd_bridge_register(dev, > to_of_node(fwnode)); > + alt_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, > to_of_node(fwnode)); > if (IS_ERR(alt_port->bridge)) { > fwnode_handle_put(fwnode); > return PTR_ERR(alt_port->bridge); > @@ -510,6 +510,16 @@ static int pmic_glink_altmode_probe(struct > auxiliary_device *adev, > } > } > > + for (port = 0; port < ARRAY_SIZE(altmode->ports); port++) { > + alt_port = >ports[port]; > + if (!alt_port->bridge) > + continue; > + > + ret = devm_drm_dp_hpd_bridge_add(dev, alt_port->bridge); > + if (ret) > + return ret; > + } > + > altmode->client = devm_pmic_glink_register_client(dev, > altmode->owner_id, > > pmic_glink_altmode_callback, > -- > 2.43.0 >
Re: [PATCH 2/6] drm/bridge: aux-hpd: separate allocation and registration
On Sat, Feb 17, 2024 at 04:02:24PM +0100, Johan Hovold wrote: > diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c > b/drivers/gpu/drm/bridge/aux-hpd-bridge.c [..] > +/** > + * devm_drm_dp_hpd_bridge_add - register a HDP DisplayPort bridge kernel-doc wants () after function names. > + * @dev: struct device to tie registration lifetime to > + * @adev: bridge auxiliary device to be registered > + * > + * Returns: zero on success or a negative errno and "Return:" without the 's'. This could however be done in a separate patch, as the file is already wrong in this regard. Reviewed-by: Bjorn Andersson Regards, Bjorn
linux-next: build failure after merge of the drm-misc tree
Hi all, After merging the drm-misc tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:47:6: error: redefinition of 'rzg2l_du_vsp_enable' 47 | void rzg2l_du_vsp_enable(struct rzg2l_du_crtc *crtc) | ^~~ In file included from drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h:18, from drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:30: drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:72:20: note: previous definition of 'rzg2l_du_vsp_enable' with type 'void(struct rzg2l_du_crtc *)' 72 | static inline void rzg2l_du_vsp_enable(struct rzg2l_du_crtc *crtc) { }; |^~~ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:61:6: error: redefinition of 'rzg2l_du_vsp_disable' 61 | void rzg2l_du_vsp_disable(struct rzg2l_du_crtc *crtc) | ^~~~ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:73:20: note: previous definition of 'rzg2l_du_vsp_disable' with type 'void(struct rzg2l_du_crtc *)' 73 | static inline void rzg2l_du_vsp_disable(struct rzg2l_du_crtc *crtc) { }; |^~~~ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:66:6: error: redefinition of 'rzg2l_du_vsp_atomic_flush' 66 | void rzg2l_du_vsp_atomic_flush(struct rzg2l_du_crtc *crtc) | ^ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:74:20: note: previous definition of 'rzg2l_du_vsp_atomic_flush' with type 'void(struct rzg2l_du_crtc *)' 74 | static inline void rzg2l_du_vsp_atomic_flush(struct rzg2l_du_crtc *crtc) { }; |^ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:76:19: error: redefinition of 'rzg2l_du_vsp_get_drm_plane' 76 | struct drm_plane *rzg2l_du_vsp_get_drm_plane(struct rzg2l_du_crtc *crtc, | ^~ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:75:33: note: previous definition of 'rzg2l_du_vsp_get_drm_plane' with type 'struct drm_plane *(struct rzg2l_du_crtc *, unsigned int)' 75 | static inline struct drm_plane *rzg2l_du_vsp_get_drm_plane(struct rzg2l_du_crtc *crtc, | ^~ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:302:5: error: redefinition of 'rzg2l_du_vsp_init' 302 | int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp, struct device_node *np, | ^ drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:66:19: note: previous definition of 'rzg2l_du_vsp_init' with type 'int(struct rzg2l_du_vsp *, struct device_node *, unsigned int)' 66 | static inline int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp, struct device_node *np, | ^ Caused by commit 768e9e61b3b9 ("drm: renesas: Add RZ/G2L DU Support") I have used the drm-misc tree from next-20240221 for today. -- Cheers, Stephen Rothwell pgpPN1_TME7xx.pgp Description: OpenPGP digital signature
Re: [PATCH 1/6] drm/bridge: aux-hpd: fix OF node leaks
On Sat, Feb 17, 2024 at 04:02:23PM +0100, Johan Hovold wrote: > The two device node references taken during allocation need to be > dropped when the auxiliary device is freed. > > Fixes: 6914968a0b52 ("drm/bridge: properly refcount DT nodes in aux bridge > drivers") > Cc: Dmitry Baryshkov > Cc: Neil Armstrong > Signed-off-by: Johan Hovold Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/gpu/drm/bridge/aux-hpd-bridge.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c > b/drivers/gpu/drm/bridge/aux-hpd-bridge.c > index bb55f697a181..9e71daf95bde 100644 > --- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c > +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c > @@ -25,6 +25,7 @@ static void drm_aux_hpd_bridge_release(struct device *dev) > ida_free(_aux_hpd_bridge_ida, adev->id); > > of_node_put(adev->dev.platform_data); > + of_node_put(adev->dev.of_node); > > kfree(adev); > } > @@ -74,6 +75,8 @@ struct device *drm_dp_hpd_bridge_register(struct device > *parent, > > ret = auxiliary_device_init(adev); > if (ret) { > + of_node_put(adev->dev.platform_data); > + of_node_put(adev->dev.of_node); > ida_free(_aux_hpd_bridge_ida, adev->id); > kfree(adev); > return ERR_PTR(ret); > -- > 2.43.0 >
[PATCH v2] drivers/i915/intel_bios: Fix parsing backlight BDB data
Starting BDB version 239, hdr_dpcd_refresh_timeout is introduced to backlight BDB data. Commit 700034566d68 ("drm/i915/bios: Define more BDB contents") updated the backlight BDB data accordingly. This broke the parsing of backlight BDB data in VBT for versions 236 - 238 (both inclusive) and hence the backlight controls are not responding on units with the concerned BDB version. backlight_control information has been present in backlight BDB data from at least BDB version 191 onwards, if not before. Hence this patch extracts the backlight_control information for BDB version 191 or newer. Tested on Chromebooks using Jasperlake SoC (reports bdb->version = 236). Tested on Chromebooks using Raptorlake SoC (reports bdb->version = 251). Fixes: 700034566d68 ("drm/i915/bios: Define more BDB contents") Cc: sta...@vger.kernel.org Cc: Jani Nikula Cc: Ville Syrjälä Signed-off-by: Karthikeyan Ramasubramanian --- Changes in v2: - removed checking the block size of the backlight BDB data drivers/gpu/drm/i915/display/intel_bios.c | 19 --- drivers/gpu/drm/i915/display/intel_vbt_defs.h | 5 - 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index aa169b0055e97..8c1eb05fe77d2 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -1042,22 +1042,11 @@ parse_lfp_backlight(struct drm_i915_private *i915, panel->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI; panel->vbt.backlight.controller = 0; if (i915->display.vbt.version >= 191) { - size_t exp_size; + const struct lfp_backlight_control_method *method; - if (i915->display.vbt.version >= 236) - exp_size = sizeof(struct bdb_lfp_backlight_data); - else if (i915->display.vbt.version >= 234) - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_234; - else - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_191; - - if (get_blocksize(backlight_data) >= exp_size) { - const struct lfp_backlight_control_method *method; - - method = _data->backlight_control[panel_type]; - panel->vbt.backlight.type = method->type; - panel->vbt.backlight.controller = method->controller; - } + method = _data->backlight_control[panel_type]; + panel->vbt.backlight.type = method->type; + panel->vbt.backlight.controller = method->controller; } panel->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz; diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h index a9f44abfc9fc2..b50cd0dcabda9 100644 --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h @@ -897,11 +897,6 @@ struct lfp_brightness_level { u16 reserved; } __packed; -#define EXP_BDB_LFP_BL_DATA_SIZE_REV_191 \ - offsetof(struct bdb_lfp_backlight_data, brightness_level) -#define EXP_BDB_LFP_BL_DATA_SIZE_REV_234 \ - offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits) - struct bdb_lfp_backlight_data { u8 entry_size; struct lfp_backlight_data_entry data[16]; -- 2.44.0.rc0.258.g7320e95886-goog
[PATCH] gpu: host1x: Skip reset assert on Tegra186
From: Mikko Perttunen On Tegra186, secure world applications may need to access host1x during suspend/resume, and rely on the kernel to keep Host1x out of reset during the suspend cycle. As such, as a quirk, skip asserting Host1x's reset on Tegra186. We don't need to keep the clocks enabled, as BPMP ensures the clock stays on while Host1x is being used. On newer SoC's, the reset line is inaccessible, so there is no need for the quirk. Signed-off-by: Mikko Perttunen --- drivers/gpu/host1x/dev.c | 15 +-- drivers/gpu/host1x/dev.h | 6 ++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index 42fd504abbcd..89983d7d73ca 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -169,6 +169,7 @@ static const struct host1x_info host1x06_info = { .num_sid_entries = ARRAY_SIZE(tegra186_sid_table), .sid_table = tegra186_sid_table, .reserve_vblank_syncpts = false, + .skip_reset_assert = true, }; static const struct host1x_sid_entry tegra194_sid_table[] = { @@ -680,13 +681,15 @@ static int __maybe_unused host1x_runtime_suspend(struct device *dev) host1x_intr_stop(host); host1x_syncpt_save(host); - err = reset_control_bulk_assert(host->nresets, host->resets); - if (err) { - dev_err(dev, "failed to assert reset: %d\n", err); - goto resume_host1x; - } + if (!host->info->skip_reset_assert) { + err = reset_control_bulk_assert(host->nresets, host->resets); + if (err) { + dev_err(dev, "failed to assert reset: %d\n", err); + goto resume_host1x; + } - usleep_range(1000, 2000); + usleep_range(1000, 2000); + } clk_disable_unprepare(host->clk); reset_control_bulk_release(host->nresets, host->resets); diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h index c8e302de7625..6143c2a61d70 100644 --- a/drivers/gpu/host1x/dev.h +++ b/drivers/gpu/host1x/dev.h @@ -116,6 +116,12 @@ struct host1x_info { * the display driver disables VBLANK increments. */ bool reserve_vblank_syncpts; + /* +* On T186, secure world applications may require access to host1x +* during suspend/resume. To allow this, we need to leave host1x +* not in reset. +*/ + bool skip_reset_assert; }; struct host1x { -- 2.42.0
Re: [PATCH v2 1/3] drm/syncobj: call drm_syncobj_fence_add_wait when WAIT_AVAILABLE flag is set
Can someone pick these up into misc? Dave. On Thu, 22 Feb 2024 at 04:48, Erik Kurzinger wrote: > > It looks these these patches have still not been merged after one month, is > there anything more that needs to be done for this to happen? > > On 1/25/24 10:12, Daniel Vetter wrote: > > On Fri, Jan 19, 2024 at 08:32:06AM -0800, Erik Kurzinger wrote: > >> When waiting for a syncobj timeline point whose fence has not yet been > >> submitted with the WAIT_FOR_SUBMIT flag, a callback is registered using > >> drm_syncobj_fence_add_wait and the thread is put to sleep until the > >> timeout expires. If the fence is submitted before then, > >> drm_syncobj_add_point will wake up the sleeping thread immediately which > >> will proceed to wait for the fence to be signaled. > >> > >> However, if the WAIT_AVAILABLE flag is used instead, > >> drm_syncobj_fence_add_wait won't get called, meaning the waiting thread > >> will always sleep for the full timeout duration, even if the fence gets > >> submitted earlier. If it turns out that the fence *has* been submitted > >> by the time it eventually wakes up, it will still indicate to userspace > >> that the wait completed successfully (it won't return -ETIME), but it > >> will have taken much longer than it should have. > >> > >> To fix this, we must call drm_syncobj_fence_add_wait if *either* the > >> WAIT_FOR_SUBMIT flag or the WAIT_AVAILABLE flag is set. The only > >> difference being that with WAIT_FOR_SUBMIT we will also wait for the > >> fence to be signaled after it has been submitted while with > >> WAIT_AVAILABLE we will return immediately. > >> > >> IGT test patch: > >> https://lists.freedesktop.org/archives/igt-dev/2024-January/067537.html > >> > >> v1 -> v2: adjust lockdep_assert_none_held_once condition > >> > >> Fixes: 01d6c3578379 ("drm/syncobj: add support for timeline point wait v8") > >> Signed-off-by: Erik Kurzinger > > > > Yeah I think this series catches now all the corner cases I spotted in v1. > > On the series: > > > > Reviewed-by: Daniel Vetter > >> --- > >> drivers/gpu/drm/drm_syncobj.c | 6 -- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > >> index 94ebc71e5be5..97be8b140599 100644 > >> --- a/drivers/gpu/drm/drm_syncobj.c > >> +++ b/drivers/gpu/drm/drm_syncobj.c > >> @@ -1058,7 +1058,8 @@ static signed long > >> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, > >> uint64_t *points; > >> uint32_t signaled_count, i; > >> > >> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) > >> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | > >> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) > >> lockdep_assert_none_held_once(); > >> > >> points = kmalloc_array(count, sizeof(*points), GFP_KERNEL); > >> @@ -1127,7 +1128,8 @@ static signed long > >> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, > >> * fallthough and try a 0 timeout wait! > >> */ > >> > >> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { > >> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | > >> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) { > >> for (i = 0; i < count; ++i) > >> drm_syncobj_fence_add_wait(syncobjs[i], [i]); > >> } > >> -- > >> 2.43.0 > >> > > >
Re: [PATCH 7/9] arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > The RB3gen2 has a USB redriver on APPS_I2C, enable the bus and introduce > the redriver. The plumbing with other components is kept separate for > clarity. > > Signed-off-by: Bjorn Andersson > --- > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > With the ADSP remoteproc loaded pmic_glink can be introduced and wired > up to provide role and orientation switching signals. > > Signed-off-by: Bjorn Andersson > --- > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48 > +++- > 1 file changed, 47 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > index ab498494caea..079bf43b14cc 100644 > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > @@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 { > }; > }; > > + pmic-glink { > + compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + connector@0 { > + compatible = "usb-c-connector"; > + reg = <0>; > + power-role = "dual"; > + data-role = "dual"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + pmic_glink_hs_in: endpoint { > + remote-endpoint = > <_1_dwc3_hs>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + pmic_glink_ss_in: endpoint { > + remote-endpoint = > <_1_dwc3_ss>; This should be connected to the QMP PHY rather than to the USB host. Also it might be better to squash this patch with the patch 8. Or at least to get redriver into the picture in this patch (and keep only display-related parts in that patch). > + }; > + }; > + }; > + }; > + }; > + > vph_pwr: vph-pwr-regulator { > compatible = "regulator-fixed"; > regulator-name = "vph_pwr"; > @@ -476,7 +511,16 @@ _1 { > }; > > _1_dwc3 { > - dr_mode = "peripheral"; > + dr_mode = "otg"; > + usb-role-switch; > +}; > + > +_1_dwc3_hs { > + remote-endpoint = <_glink_hs_in>; > +}; > + > +_1_dwc3_ss { > + remote-endpoint = <_glink_ss_in>; > }; > > _1_hsphy { > @@ -491,6 +535,8 @@ _1_qmpphy { > vdda-phy-supply = <_l6b_1p2>; > vdda-pll-supply = <_l1b_0p912>; > > + orientation-switch; > + > status = "okay"; > }; > > > -- > 2.25.1 > -- With best wishes Dmitry
Re: [PATCH 4/9] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > The RB3Gen2 board comes with a mini DP connector, describe this, enable > MDSS, DP controller and the PHY that drives this. > > Signed-off-by: Bjorn Andersson > --- > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > index ac4579119d3b..32313f47602a 100644 > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > @@ -430,6 +430,23 @@ { >; > }; > > + { > + status = "okay"; > +}; > + > +_edp { > + status = "okay"; > +}; > + > +_edp_out { > + data-lanes = <0 1 2 3>; > + link-frequencies = /bits/ 64 <162000 27 54 > 81>; > +}; Please add a corresponding dp-connector device and link it to the mdss_edp_out. > + > +_edp_phy { > + status = "okay"; > +}; > + > _id_0 { > status = "okay"; > }; > @@ -470,3 +487,9 @@ _1_qmpphy { > { > memory-region = <_fw_mem>; > }; > + > +/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */ > + > +_hot_plug_det { > + bias-disable; > +}; > > -- > 2.25.1 > -- With best wishes Dmitry
Re: [PATCH 9/9] arm64: defconfig: Enable sc7280 display and gpu clock controllers
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > Enable the SC7280 display and gpu clock controllers to enable display > support on the QCS6490 RB3gen2. > > Signed-off-by: Bjorn Andersson > --- > arch/arm64/configs/defconfig | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 2/9] arm64: dts: qcom: sc7280: Make eDP/DP controller default DP
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > The newly introduced mechanism for selecting eDP mode allow us to make a > DisplayPort controller operate in eDP mode, but not the other way > around. The qcom,sc7280-edp compatible is obviously tied to eDP, so this > would not allow us to select DisplayPort-mode. > > Switch the compatible of the mdss_edp instance and make it eDP for the > SC7280 qcard. > > Signed-off-by: Bjorn Andersson > --- > arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 ++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 5/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > Define firmware paths and enable the ADSP and CDSP remoteprocs. > > Signed-off-by: Bjorn Andersson > --- > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > index 32313f47602a..ab498494caea 100644 > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts > @@ -451,6 +451,16 @@ _id_0 { > status = "okay"; > }; > > +_adsp { > + firmware-name = "qcom/qcs6490/rb3gen2/adsp.mbn"; This should be either firmware-name = "qcom/qcs6490/adsp.mbn"; or firmware-name = "qcom/qcs6490/Vendor/rb3gen2/adsp.mbn"; > + status = "okay"; > +}; > + > +_cdsp { > + firmware-name = "qcom/qcs6490/rb3gen2/cdsp.mbn"; > + status = "okay"; > +}; > + > { > gpio-reserved-ranges = <32 2>, /* ADSP */ ><48 4>; /* NFC */ > > -- > 2.25.1 > -- With best wishes Dmitry
Re: [PATCH 3/9] arm64: dts: qcom: sc7280: Enable MDP turbo mode
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > The max frequency listed in the DPU opp-table is 506MHz, this is not > sufficient to drive a 4k@60 display, resulting in constant underrun. > > Add the missing MDP_CLK turbo frequency of 608MHz to the opp-table to > fix this. I think we might want to keep this disabled for ChromeOS devices. Doug? > > Signed-off-by: Bjorn Andersson > --- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi > b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index a19c278ebec9..a2a6717c6c87 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -4417,6 +4417,11 @@ opp-50667 { > opp-hz = /bits/ 64 > <50667>; > required-opps = > <_opp_nom>; > }; > + > + opp-60800 { > + opp-hz = /bits/ 64 > <60800>; > + required-opps = > <_opp_turbo>; > + }; > }; > }; > > > -- > 2.25.1 > -- With best wishes Dmitry
Re: [PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280
On Thu, 22 Feb 2024 at 01:19, Bjorn Andersson wrote: > > When upstreamed the SC7280 DP controllers where described as one being > DP and one eDP, but they can infact both be DP or eDP. > > Extend the list of DP controllers to cover both instances, and rely on > the newly introduced mechanism for selecting which mode they should > operate in. > > Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree > will continue to select eDP. > > Signed-off-by: Bjorn Andersson > --- > drivers/gpu/drm/msm/dp/dp_display.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index 7b8c695d521a..1792ba9f7259 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = { > }; > > static const struct msm_dp_desc sc7280_dp_descs[] = { > - { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type > = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true }, > + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = > true }, > + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = > true }, I think we need to keep .connector_type here, don't we? > + {} > +}; > + > +static const struct msm_dp_desc sc7280_edp_descs[] = { > { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type > = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true }, > {} > }; > @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = { > static const struct of_device_id dp_dt_match[] = { > { .compatible = "qcom,sc7180-dp", .data = _dp_descs }, > { .compatible = "qcom,sc7280-dp", .data = _dp_descs }, > - { .compatible = "qcom,sc7280-edp", .data = _dp_descs }, > + { .compatible = "qcom,sc7280-edp", .data = _edp_descs }, > { .compatible = "qcom,sc8180x-dp", .data = _dp_descs }, > { .compatible = "qcom,sc8180x-edp", .data = _dp_descs }, > { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs }, > > -- > 2.25.1 > -- With best wishes Dmitry
[PATCH 4/9] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output
The RB3Gen2 board comes with a mini DP connector, describe this, enable MDSS, DP controller and the PHY that drives this. Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 23 +++ 1 file changed, 23 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts index ac4579119d3b..32313f47602a 100644 --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts @@ -430,6 +430,23 @@ { ; }; + { + status = "okay"; +}; + +_edp { + status = "okay"; +}; + +_edp_out { + data-lanes = <0 1 2 3>; + link-frequencies = /bits/ 64 <162000 27 54 81>; +}; + +_edp_phy { + status = "okay"; +}; + _id_0 { status = "okay"; }; @@ -470,3 +487,9 @@ _1_qmpphy { { memory-region = <_fw_mem>; }; + +/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */ + +_hot_plug_det { + bias-disable; +}; -- 2.25.1
[PATCH 9/9] arm64: defconfig: Enable sc7280 display and gpu clock controllers
Enable the SC7280 display and gpu clock controllers to enable display support on the QCS6490 RB3gen2. Signed-off-by: Bjorn Andersson --- arch/arm64/configs/defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index b8adb28185ad..193d504041dd 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1283,6 +1283,7 @@ CONFIG_QCM_DISPCC_2290=m CONFIG_QCS_GCC_404=y CONFIG_QDU_GCC_1000=y CONFIG_SC_CAMCC_8280XP=m +CONFIG_SC_DISPCC_7280=m CONFIG_SC_DISPCC_8280XP=m CONFIG_SA_GCC_8775P=y CONFIG_SA_GPUCC_8775P=m @@ -1290,6 +1291,7 @@ CONFIG_SC_GCC_7180=y CONFIG_SC_GCC_7280=y CONFIG_SC_GCC_8180X=y CONFIG_SC_GCC_8280XP=y +CONFIG_SC_GPUCC_7280=m CONFIG_SC_GPUCC_8280XP=m CONFIG_SC_LPASSCC_8280XP=m CONFIG_SDM_CAMCC_845=m -- 2.25.1
[PATCH 1/9] drm/msm/dp: Add DP support to combo instance in SC7280
When upstreamed the SC7280 DP controllers where described as one being DP and one eDP, but they can infact both be DP or eDP. Extend the list of DP controllers to cover both instances, and rely on the newly introduced mechanism for selecting which mode they should operate in. Move qcom,sc7280-edp to a dedicated list, to ensure existing DeviceTree will continue to select eDP. Signed-off-by: Bjorn Andersson --- drivers/gpu/drm/msm/dp/dp_display.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 7b8c695d521a..1792ba9f7259 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -129,7 +129,12 @@ static const struct msm_dp_desc sc7180_dp_descs[] = { }; static const struct msm_dp_desc sc7280_dp_descs[] = { - { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true }, + { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = true }, + { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = true }, + {} +}; + +static const struct msm_dp_desc sc7280_edp_descs[] = { { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true }, {} }; @@ -182,7 +187,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = { static const struct of_device_id dp_dt_match[] = { { .compatible = "qcom,sc7180-dp", .data = _dp_descs }, { .compatible = "qcom,sc7280-dp", .data = _dp_descs }, - { .compatible = "qcom,sc7280-edp", .data = _dp_descs }, + { .compatible = "qcom,sc7280-edp", .data = _edp_descs }, { .compatible = "qcom,sc8180x-dp", .data = _dp_descs }, { .compatible = "qcom,sc8180x-edp", .data = _dp_descs }, { .compatible = "qcom,sc8280xp-dp", .data = _dp_descs }, -- 2.25.1
[PATCH 5/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp
Define firmware paths and enable the ADSP and CDSP remoteprocs. Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts index 32313f47602a..ab498494caea 100644 --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts @@ -451,6 +451,16 @@ _id_0 { status = "okay"; }; +_adsp { + firmware-name = "qcom/qcs6490/rb3gen2/adsp.mbn"; + status = "okay"; +}; + +_cdsp { + firmware-name = "qcom/qcs6490/rb3gen2/cdsp.mbn"; + status = "okay"; +}; + { gpio-reserved-ranges = <32 2>, /* ADSP */ <48 4>; /* NFC */ -- 2.25.1
[PATCH 8/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB Type-C display
With MDSS, pmic_glink, and the redriver in place, wire up the various components to enable USB Type-C display on the RB3gen2. Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 63 +++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts index 171ed979d55f..4bf1c6351467 100644 --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts @@ -149,7 +149,15 @@ port@1 { reg = <1>; pmic_glink_ss_in: endpoint { - remote-endpoint = <_1_dwc3_ss>; + remote-endpoint = <_usb_con_ss>; + }; + }; + + port@2 { + reg = <2>; + + pmic_glink_sbu_in: endpoint { + remote-endpoint = <_usb_con_sbu>; }; }; }; @@ -476,6 +484,36 @@ typec-mux@1c { retimer-switch; orientation-switch; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + redriver_usb_con_ss: endpoint { + remote-endpoint = <_glink_ss_in>; + }; + }; + + port@1 { + reg = <1>; + + redriver_phy_con_ss: endpoint { + remote-endpoint = <_dp_qmpphy_out>; + data-lanes = <0 1 2 3>; + }; + }; + + port@2 { + reg = <2>; + + redriver_usb_con_sbu: endpoint { + remote-endpoint = <_glink_sbu_in>; + }; + }; + }; }; }; @@ -483,6 +521,15 @@ { status = "okay"; }; +_dp { + status = "okay"; +}; + +_dp_out { + data-lanes = <0 1>; + remote-endpoint = <_dp_qmpphy_dp_in>; +}; + _edp { status = "okay"; }; @@ -534,7 +581,7 @@ _1_dwc3_hs { }; _1_dwc3_ss { - remote-endpoint = <_glink_ss_in>; + remote-endpoint = <_dp_qmpphy_usb_ss_in>; }; _1_hsphy { @@ -554,6 +601,18 @@ _1_qmpphy { status = "okay"; }; +_dp_qmpphy_out { + remote-endpoint = <_phy_con_ss>; +}; + +_dp_qmpphy_usb_ss_in { + remote-endpoint = <_1_dwc3_ss>; +}; + +_dp_qmpphy_dp_in { + remote-endpoint = <_dp_out>; +}; + { memory-region = <_fw_mem>; }; -- 2.25.1
[PATCH 2/9] arm64: dts: qcom: sc7280: Make eDP/DP controller default DP
The newly introduced mechanism for selecting eDP mode allow us to make a DisplayPort controller operate in eDP mode, but not the other way around. The qcom,sc7280-edp compatible is obviously tied to eDP, so this would not allow us to select DisplayPort-mode. Switch the compatible of the mdss_edp instance and make it eDP for the SC7280 qcard. Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 ++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi index f9b96bd2477e..e339b181a9ac 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi @@ -348,6 +348,8 @@ _va_macro { /* NOTE: Not all Qcards have eDP connector stuffed */ _edp { + is-edp; + aux-bus { edp_panel: panel { compatible = "edp-panel"; diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 581818676a4c..a19c278ebec9 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -4513,7 +4513,7 @@ mdss_dsi_phy: phy@ae94400 { }; mdss_edp: edp@aea { - compatible = "qcom,sc7280-edp"; + compatible = "qcom,sc7280-dp"; pinctrl-names = "default"; pinctrl-0 = <_hot_plug_det>; -- 2.25.1
[PATCH 7/9] arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver
The RB3gen2 has a USB redriver on APPS_I2C, enable the bus and introduce the redriver. The plumbing with other components is kept separate for clarity. Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 14 ++ 1 file changed, 14 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts index 079bf43b14cc..171ed979d55f 100644 --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts @@ -465,6 +465,20 @@ { ; }; + { + status = "okay"; + + typec-mux@1c { + compatible = "onnn,nb7vpq904m"; + reg = <0x1c>; + + vcc-supply = <_l18b_1p8>; + + retimer-switch; + orientation-switch; + }; +}; + { status = "okay"; }; -- 2.25.1
[PATCH 6/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching
With the ADSP remoteproc loaded pmic_glink can be introduced and wired up to provide role and orientation switching signals. Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 48 +++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts index ab498494caea..079bf43b14cc 100644 --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts @@ -121,6 +121,41 @@ debug_vm_mem: debug-vm@d060 { }; }; + pmic-glink { + compatible = "qcom,qcm6490-pmic-glink", "qcom,pmic-glink"; + + #address-cells = <1>; + #size-cells = <0>; + + connector@0 { + compatible = "usb-c-connector"; + reg = <0>; + power-role = "dual"; + data-role = "dual"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + pmic_glink_hs_in: endpoint { + remote-endpoint = <_1_dwc3_hs>; + }; + }; + + port@1 { + reg = <1>; + + pmic_glink_ss_in: endpoint { + remote-endpoint = <_1_dwc3_ss>; + }; + }; + }; + }; + }; + vph_pwr: vph-pwr-regulator { compatible = "regulator-fixed"; regulator-name = "vph_pwr"; @@ -476,7 +511,16 @@ _1 { }; _1_dwc3 { - dr_mode = "peripheral"; + dr_mode = "otg"; + usb-role-switch; +}; + +_1_dwc3_hs { + remote-endpoint = <_glink_hs_in>; +}; + +_1_dwc3_ss { + remote-endpoint = <_glink_ss_in>; }; _1_hsphy { @@ -491,6 +535,8 @@ _1_qmpphy { vdda-phy-supply = <_l6b_1p2>; vdda-pll-supply = <_l1b_0p912>; + orientation-switch; + status = "okay"; }; -- 2.25.1
[PATCH 3/9] arm64: dts: qcom: sc7280: Enable MDP turbo mode
The max frequency listed in the DPU opp-table is 506MHz, this is not sufficient to drive a 4k@60 display, resulting in constant underrun. Add the missing MDP_CLK turbo frequency of 608MHz to the opp-table to fix this. Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index a19c278ebec9..a2a6717c6c87 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -4417,6 +4417,11 @@ opp-50667 { opp-hz = /bits/ 64 <50667>; required-opps = <_opp_nom>; }; + + opp-60800 { + opp-hz = /bits/ 64 <60800>; + required-opps = <_opp_turbo>; + }; }; }; -- 2.25.1
[PATCH 0/9] arm64: dts: qcom: qcs6490-rb3gen2: Enable two displays
RB3Gen2 is capable of producing DisplayPort output on a dedicated mini-DP connector and USB Type-C. Utilize Abel's work for DP vs eDP selection to allow configuring both controllers in DP-mode, then enable the two output paths. Tested by driving fbcon to 4k@60 + 4k@30 concurrently. Depends on https://lore.kernel.org/linux-arm-msm/20240220-x1e80100-display-v4-0-971afd9de...@linaro.org/ Signed-off-by: Bjorn Andersson --- Bjorn Andersson (9): drm/msm/dp: Add DP support to combo instance in SC7280 arm64: dts: qcom: sc7280: Make eDP/DP controller default DP arm64: dts: qcom: sc7280: Enable MDP turbo mode arm64: dts: qcom: qcs6490-rb3gen2: Add DP output arm64: dts: qcom: qcs6490-rb3gen2: Enable adsp and cdsp arm64: dts: qcom: qcs6490-rb3gen2: Enable USB role switching arm64: dts: qcom: qcs6490-rb3gen2: Introduce USB redriver arm64: dts: qcom: qcs6490-rb3gen2: Enable USB Type-C display arm64: defconfig: Enable sc7280 display and gpu clock controllers arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 154 ++- arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 + arch/arm64/boot/dts/qcom/sc7280.dtsi | 7 +- arch/arm64/configs/defconfig | 2 + drivers/gpu/drm/msm/dp/dp_display.c | 9 +- 5 files changed, 170 insertions(+), 4 deletions(-) --- base-commit: aba508318eec7acad2373296279d6447fd35f83f change-id: 20240209-rb3gen2-dp-connector-bddfb892ff20 Best regards, -- Bjorn Andersson
Re: [PATCH] drm/amd/display: Use kcalloc() instead of kzalloc()
On 1/28/24 02:04, Lenko Donchev wrote: We are trying to get rid of all multiplications from allocation functions to prevent integer overflows. Here the multiplication is obviously safe, but using kcalloc() is more appropriate and improves readability. This patch has no effect on runtime behavior. Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments Link: https://github.com/KSPP/linux/issues/162 Signed-off-by: Lenko Donchev --- drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c index 5c9a30211c10..b67cd78e7c58 100644 --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c @@ -164,7 +164,7 @@ static void dpcd_extend_address_range( if (new_addr_range.start != in_address || new_addr_range.end != end_address) { *out_address = new_addr_range.start; *out_size = ADDRESS_RANGE_SIZE(new_addr_range.start, new_addr_range.end); - *out_data = kzalloc(*out_size * sizeof(**out_data), GFP_KERNEL); + *out_data = kcalloc(*out_size, sizeof(**out_data), GFP_KERNEL); } } lgtm, Reviewed-by: Rodrigo Siqueira
Re: Future handling of complex RGB devices on Linux v2
Hi! > so after more feedback from the OpenRGB maintainers I came up with an even > more generic proposal: > https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916#note_1753072869 > >evaluate-set-command ioctl taking: > >{ > > enum command /* one of supported_commands */ > > union data > > { > > char raw[3072], > > { > > > > }, Yeah, so ... this is not a interface. This is a backdoor to pass arbitrary data. That's not going to fly. For keyboards, we don't need complete new interface; we reasonable extensions over existing display APIs -- keyboards are clearly 2D. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH] phy: constify of_phandle_args in xlate
On 2/17/24 01:39, Krzysztof Kozlowski wrote: The xlate callbacks are supposed to translate of_phandle_args to proper provider without modifying the of_phandle_args. Make the argument pointer to const for code safety and readability. Signed-off-by: Krzysztof Kozlowski Acked-by: Florian Fainelli #Broadcom -- Florian
Re: [PATCH] phy: constify of_phandle_args in xlate
On Sat, Feb 17, 2024 at 10:39 AM Krzysztof Kozlowski wrote: > The xlate callbacks are supposed to translate of_phandle_args to proper > provider without modifying the of_phandle_args. Make the argument > pointer to const for code safety and readability. > > Signed-off-by: Krzysztof Kozlowski (...) > drivers/pinctrl/tegra/pinctrl-tegra-xusb.c | 2 +- Acked-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH] drm/xe: skip building debugfs code for CONFIG_DEBUG_FS=n
Hi Lucas, On Wed, 21 Feb 2024 13:45:35 -0600 Lucas De Marchi wrote: > > I don't think it's needed since drm-xe-next covers what is in > drm-xe-fixes. Please add other maintainers and mailing list: > > M: Oded Gabbay > M: Thomas Hellström > L: intel...@lists.freedesktop.org Added. -- Cheers, Stephen Rothwell pgpjHbRARM1Ui.pgp Description: OpenPGP digital signature
Re: Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Wed, Feb 21, 2024 at 11:04:22PM +0200, Andy Shevchenko wrote: On Wed, Feb 21, 2024 at 10:30:02PM +0200, Dmitry Baryshkov wrote: On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi wrote: ... > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) Can sizeof() be used in assembly? ... > -#define __GENMASK(h, l) \ > - (((~UL(0)) - (UL(1) << (l)) + 1) & \ > -(~UL(0) >> (BITS_PER_LONG - 1 - (h > -#define GENMASK(h, l) \ > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)) > +#define __GENMASK(t, h, l) \ > + (GENMASK_INPUT_CHECK(h, l) + \ > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \ > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h) Nevertheless, the use ~0ULL is not proper assembly, this broke initial implementation using UL() / ULL(). indeed. > -#define __GENMASK_ULL(h, l) \ > - (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h > -#define GENMASK_ULL(h, l) \ > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l)) Ditto. problem here seems actually because of the cast to the final type. My previous impl was avoiding that, but was too verbose compared to this. I will look at reverting this. Lucas De Marchi > +#define GENMASK(h, l) __GENMASK(unsigned long, h, l) > +#define GENMASK_ULL(h, l) __GENMASK(unsigned long long, h, l) > +#define GENMASK_U8(h, l) __GENMASK(u8, h, l) > +#define GENMASK_U16(h, l) __GENMASK(u16, h, l) > +#define GENMASK_U32(h, l) __GENMASK(u32, h, l) > +#define GENMASK_U64(h, l) __GENMASK(u64, h, l) This breaks drm-tip on arm64 architecture: arch/arm64/kernel/entry-fpsimd.S: Assembler messages: 465arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 466arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 467arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 468arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 469arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 470arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 471arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 472arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 473arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)' 474arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 475arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 476arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 477arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 478arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 479arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 480arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 481arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 482arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 483arch/arm64/kernel/entry-fpsimd.S:282: Error: unexpected characters following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)' 484arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here -- With Best Regards, Andy Shevchenko
Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names
On Wed, Feb 21, 2024 at 10:33:53PM +0100, Javier Carrasco wrote: > On 21.02.24 22:18, Matthias Kaehlcke wrote: > +/* > + * Fallback supply names for backwards compatibility. If the device > requires > + * more than the currently supported supplies, add a new one here, and > if > + * possible, the real name supplies to the device-specific data. > + */ > +static const char * const generic_supply_names[] = { > +"vdd", > +"vdd2", > +}; > + > +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names) > >>> > >>> This will have to change when support for a device with more than 2 > >>> non-generic > >>> supply names gets added. Please use a literal value for MAX_SUPPLIES > >>> instead of > >>> ARRAY_SIZE. If the literal is 2 it would still need to change for future > >>> devices > >>> with more supplies, but that change would be more straighforward. > >>> > >> > >> I am not completely sure about this. Someone could increase MAX_SUPPLIES > >> without adding a generic name. > > > > That's perfectly fine and intended. MAX_SUPPLIES is a max, any list > > shorther than that is valid. Any longer list will result in probe() > > being aborted with a clear error message. > > > >> Actually two modifications will be necessary for every addition (name > >> and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required, > >> and MAX_SUPPLIES is automatically increased. > > > > As per above it's not necessary to add a new name when MAX_SUPPLIES is > > increased to support more non-generic names. It would only be necessary > > if more generic names were added, my understanding is that this > > should not happen because any newly supported onboard devices are > > supposed to use device specific supply names. I don't like to idea of > > adding unused pseudo supply names to the list, just for the sake of > > using ARRAY_SIZE. > > > >> I understand that the whole point of this is getting rid of the generic > >> names, but we still have to provide generic names for every extra > >> supply, at least for code consistency and to avoid size mismatches > >> between real an generic supply names. > > > > Please let me know if you still think the extra names are needed. > > Not really, the only case I could come up is if an existing device that > uses generic names might end up requiring a third supply, which would > also be generic. But even such an unlikely event would be cover without > ARRAY_SIZE. > > Actually one could argue that every existing device could have "vdd" and > "vdd2" as their supply names and remove checks and the generic array. Sounds good to me. Another similar option would be to assign 'generic_supply_names' to '.supply_names'. I don't have a strong preference.
Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names
On 21.02.24 22:18, Matthias Kaehlcke wrote: +/* + * Fallback supply names for backwards compatibility. If the device requires + * more than the currently supported supplies, add a new one here, and if + * possible, the real name supplies to the device-specific data. + */ +static const char * const generic_supply_names[] = { + "vdd", + "vdd2", +}; + +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names) >>> >>> This will have to change when support for a device with more than 2 >>> non-generic >>> supply names gets added. Please use a literal value for MAX_SUPPLIES >>> instead of >>> ARRAY_SIZE. If the literal is 2 it would still need to change for future >>> devices >>> with more supplies, but that change would be more straighforward. >>> >> >> I am not completely sure about this. Someone could increase MAX_SUPPLIES >> without adding a generic name. > > That's perfectly fine and intended. MAX_SUPPLIES is a max, any list > shorther than that is valid. Any longer list will result in probe() > being aborted with a clear error message. > >> Actually two modifications will be necessary for every addition (name >> and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required, >> and MAX_SUPPLIES is automatically increased. > > As per above it's not necessary to add a new name when MAX_SUPPLIES is > increased to support more non-generic names. It would only be necessary > if more generic names were added, my understanding is that this > should not happen because any newly supported onboard devices are > supposed to use device specific supply names. I don't like to idea of > adding unused pseudo supply names to the list, just for the sake of > using ARRAY_SIZE. > >> I understand that the whole point of this is getting rid of the generic >> names, but we still have to provide generic names for every extra >> supply, at least for code consistency and to avoid size mismatches >> between real an generic supply names. > > Please let me know if you still think the extra names are needed. Not really, the only case I could come up is if an existing device that uses generic names might end up requiring a third supply, which would also be generic. But even such an unlikely event would be cover without ARRAY_SIZE. Actually one could argue that every existing device could have "vdd" and "vdd2" as their supply names and remove checks and the generic array. Best regards, Javier Carrasco
Re: [PATCH 7/9] drm: tests: Fix invalid printf format specifiers in KUnit tests
Hi, On Wed, Feb 21, 2024 at 05:27:20PM +0800, David Gow wrote: > The drm_buddy_test's alloc_contiguous test used a u64 for the page size, > which was then updated to be an 'unsigned long' to avoid 64-bit > multiplication division helpers. > > However, the variable is logged by some KUNIT_ASSERT_EQ_MSG() using the > '%d' or '%llu' format specifiers, the former of which is always wrong, > and the latter is no longer correct now that ps is no longer a u64. Fix > these to all use '%lu'. > > Also, drm_mm_test calls KUNIT_FAIL() with an empty string as the > message. gcc warns if a printf format string is empty (apparently), so clang does too; under -Wformat-zero-length > give these some more detailed error messages, which should be more > useful anyway. > > Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test") > Fixes: fca7526b7d89 ("drm/tests/drm_buddy: fix build failure on 32-bit > targets") > Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit") > Signed-off-by: David Gow Reviewed-by: Justin Stitt > --- > drivers/gpu/drm/tests/drm_buddy_test.c | 14 +++--- > drivers/gpu/drm/tests/drm_mm_test.c| 6 +++--- > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c > b/drivers/gpu/drm/tests/drm_buddy_test.c > index 8a464f7f4c61..3dbfa3078449 100644 > --- a/drivers/gpu/drm/tests/drm_buddy_test.c > +++ b/drivers/gpu/drm/tests/drm_buddy_test.c > @@ -55,30 +55,30 @@ static void drm_test_buddy_alloc_contiguous(struct kunit > *test) > KUNIT_ASSERT_FALSE_MSG(test, > drm_buddy_alloc_blocks(, 0, mm_size, > ps, ps, list, 0), > -"buddy_alloc hit an error size=%d\n", > +"buddy_alloc hit an error size=%lu\n", > ps); > } while (++i < n_pages); > > KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size, > 3 * ps, ps, > , > > DRM_BUDDY_CONTIGUOUS_ALLOCATION), > -"buddy_alloc didn't error size=%d\n", 3 * ps); > +"buddy_alloc didn't error size=%lu\n", 3 * ps); > > drm_buddy_free_list(, ); > KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size, > 3 * ps, ps, > , > > DRM_BUDDY_CONTIGUOUS_ALLOCATION), > -"buddy_alloc didn't error size=%llu\n", 3 * ps); > +"buddy_alloc didn't error size=%lu\n", 3 * ps); > KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size, > 2 * ps, ps, > , > > DRM_BUDDY_CONTIGUOUS_ALLOCATION), > -"buddy_alloc didn't error size=%llu\n", 2 * ps); > +"buddy_alloc didn't error size=%lu\n", 2 * ps); > > drm_buddy_free_list(, ); > KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size, > 3 * ps, ps, > , > > DRM_BUDDY_CONTIGUOUS_ALLOCATION), > -"buddy_alloc didn't error size=%llu\n", 3 * ps); > +"buddy_alloc didn't error size=%lu\n", 3 * ps); > /* >* At this point we should have enough contiguous space for 2 blocks, >* however they are never buddies (since we freed middle and right) so > @@ -87,13 +87,13 @@ static void drm_test_buddy_alloc_contiguous(struct kunit > *test) > KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size, > 2 * ps, ps, > , > > DRM_BUDDY_CONTIGUOUS_ALLOCATION), > -"buddy_alloc hit an error size=%d\n", 2 * ps); > +"buddy_alloc hit an error size=%lu\n", 2 * ps); > > drm_buddy_free_list(, ); > KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size, > 3 * ps, ps, > , > > DRM_BUDDY_CONTIGUOUS_ALLOCATION), > -"buddy_alloc hit an error size=%d\n", 3 * ps); > +"buddy_alloc hit an error size=%lu\n", 3 * ps); > > total = 0; > list_for_each_entry(block, , link) > diff --git a/drivers/gpu/drm/tests/drm_mm_test.c > b/drivers/gpu/drm/tests/drm_mm_test.c > index 1eb0c304f960..f37c0d765865 100644 > ---
Re: [PATCH] drm/i915/guc: Add Compute context hint
On Wed, Feb 21, 2024 at 09:42:34AM +, Tvrtko Ursulin wrote: > > On 21/02/2024 00:14, Vinay Belgaumkar wrote: > > Allow user to provide a context hint. When this is set, KMD will > > send a hint to GuC which results in special handling for this > > context. SLPC will ramp the GT frequency aggressively every time > > it switches to this context. The down freq threshold will also be > > lower so GuC will ramp down the GT freq for this context more slowly. > > We also disable waitboost for this context as that will interfere with > > the strategy. > > > > We need to enable the use of Compute strategy during SLPC init, but > > it will apply only to contexts that set this bit during context > > creation. > > > > Userland can check whether this feature is supported using a new param- > > I915_PARAM_HAS_COMPUTE_CONTEXT. This flag is true for all guc submission > > enabled platforms since they use SLPC for freq management. > > > > The Mesa usage model for this flag is here - > > https://gitlab.freedesktop.org/sushmave/mesa/-/commits/compute_hint > > This allows for setting it for the whole application, correct? Upsides, > downsides? Are there any plans for per context? Currently there's no extension on a high level API (Vulkan/OpenGL/OpenCL/etc) that would allow the application to hint for power/freq/latency. So Mesa cannot decide when to hint. So their solution was to use .drirc and make per-application decision. I would prefer a high level extension for a more granular and informative decision. We need to work with that goal, but for now I don't see any cons on this approach. > > > Cc: Rodrigo Vivi > > Signed-off-by: Vinay Belgaumkar > > --- > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 8 +++ > > .../gpu/drm/i915/gem/i915_gem_context_types.h | 1 + > > drivers/gpu/drm/i915/gt/intel_rps.c | 8 +++ > > .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h | 21 +++ > > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 17 +++ > > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 1 + > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 +++ > > drivers/gpu/drm/i915/i915_getparam.c | 11 ++ > > include/uapi/drm/i915_drm.h | 15 + > > 9 files changed, 89 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > index dcbfe32fd30c..ceab7dbe9b47 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > @@ -879,6 +879,7 @@ static int set_proto_ctx_param(struct > > drm_i915_file_private *fpriv, > >struct i915_gem_proto_context *pc, > >struct drm_i915_gem_context_param *args) > > { > > + struct drm_i915_private *i915 = fpriv->i915; > > int ret = 0; > > switch (args->param) { > > @@ -904,6 +905,13 @@ static int set_proto_ctx_param(struct > > drm_i915_file_private *fpriv, > > pc->user_flags &= ~BIT(UCONTEXT_BANNABLE); > > break; > > + case I915_CONTEXT_PARAM_IS_COMPUTE: > > + if (!intel_uc_uses_guc_submission(_gt(i915)->uc)) > > + ret = -EINVAL; > > + else > > + pc->user_flags |= BIT(UCONTEXT_COMPUTE); > > + break; > > + > > case I915_CONTEXT_PARAM_RECOVERABLE: > > if (args->size) > > ret = -EINVAL; > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h > > b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h > > index 03bc7f9d191b..db86d6f6245f 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h > > @@ -338,6 +338,7 @@ struct i915_gem_context { > > #define UCONTEXT_BANNABLE 2 > > #define UCONTEXT_RECOVERABLE 3 > > #define UCONTEXT_PERSISTENCE 4 > > +#define UCONTEXT_COMPUTE 5 > > What is the GuC behaviour when SLPC_CTX_FREQ_REQ_IS_COMPUTE is set for > non-compute engines? Wondering if per intel_context is what we want instead. > (Which could then be the i915_context_param_engines extension to mark > individual contexts as compute strategy.) Perhaps we should rename this? This is a freq-decision-strategy inside GuC that is there mostly targeting compute workloads that needs lower latency with short burst execution. But the engine itself doesn't matter. It can be applied to any engine. > > > /** > > * @flags: small set of booleans > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c > > b/drivers/gpu/drm/i915/gt/intel_rps.c > > index 4feef874e6d6..1ed40cd61b70 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > > @@ -24,6 +24,7 @@ > > #include "intel_pcode.h" > > #include "intel_rps.h" > > #include "vlv_sideband.h" > > +#include "../gem/i915_gem_context.h" > >
Re: [PATCH 6/9] net: test: Fix printf format specifier in skb_segment kunit test
Hi, On Wed, Feb 21, 2024 at 05:27:19PM +0800, David Gow wrote: > KUNIT_FAIL() accepts a printf-style format string, but previously did > not let gcc validate it with the __printf() attribute. The use of %lld > for the result of PTR_ERR() is not correct. > > Instead, use %pe and pass the actual error pointer. printk() will format > it correctly (and give a symbolic name rather than a number if > available, which should make the output more readable, too). > > Fixes: b3098d32ed6e ("net: add skb_segment kunit test") > Signed-off-by: David Gow Looks good. For those wondering, %pe has a special meaning in the kernel which can be seen in lib/vsprintf.c. Reviewed-by: Justin Stitt > --- > net/core/gso_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/gso_test.c b/net/core/gso_test.c > index 4c2e77bd12f4..358c44680d91 100644 > --- a/net/core/gso_test.c > +++ b/net/core/gso_test.c > @@ -225,7 +225,7 @@ static void gso_test_func(struct kunit *test) > > segs = skb_segment(skb, features); > if (IS_ERR(segs)) { > - KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs)); > + KUNIT_FAIL(test, "segs error %pe", segs); > goto free_gso_skb; > } else if (!segs) { > KUNIT_FAIL(test, "no segments"); > -- > 2.44.0.rc0.258.g7320e95886-goog > Thanks Justin
Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names
Hi Javier, On Wed, Feb 21, 2024 at 09:40:38PM +0100, Javier Carrasco wrote: > On 21.02.24 21:25, Matthias Kaehlcke wrote: > > On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote: > >> The current mechanism uses generic names for the power supplies, which > >> conflicts with proper name definitions in the device bindings. > >> > >> Add a per-device property to include real supply names and keep generic > >> names as a fallback mechanism for backward compatibility. > >> > >> Signed-off-by: Javier Carrasco > >> --- > >> drivers/usb/misc/onboard_usb_dev.c | 54 > >> -- > >> drivers/usb/misc/onboard_usb_dev.h | 23 > >> 2 files changed, 52 insertions(+), 25 deletions(-) > >> > >> diff --git a/drivers/usb/misc/onboard_usb_dev.c > >> b/drivers/usb/misc/onboard_usb_dev.c > >> index f43130a6786f..e66fcac93006 100644 > >> --- a/drivers/usb/misc/onboard_usb_dev.c > >> +++ b/drivers/usb/misc/onboard_usb_dev.c > >> @@ -29,18 +29,6 @@ > >> > >> #include "onboard_usb_dev.h" > >> > >> -/* > >> - * Use generic names, as the actual names might differ between devices. > >> If a new > >> - * device requires more than the currently supported supplies, add a new > >> one > >> - * here. > >> - */ > >> -static const char * const supply_names[] = { > >> - "vdd", > >> - "vdd2", > >> -}; > >> - > >> -#define MAX_SUPPLIES ARRAY_SIZE(supply_names) > >> - > >> static void onboard_dev_attach_usb_driver(struct work_struct *work); > >> > >> static struct usb_device_driver onboard_dev_usbdev_driver; > >> @@ -66,6 +54,33 @@ struct onboard_dev { > >>struct clk *clk; > >> }; > >> > >> +static int onboard_dev_get_regulator_bulk(struct device *dev, > >> +struct onboard_dev *onboard_dev) > >> +{ > >> + unsigned int i; > >> + int err; > >> + > >> + const char * const *supply_names = onboard_dev->pdata->supply_names; > >> + > >> + if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES) > >> + return dev_err_probe(dev, -EINVAL, "max %zu supplies > >> supported!\n", > >> + MAX_SUPPLIES); > >> + > >> + if (!supply_names[0]) > >> + supply_names = generic_supply_names; > > > > Please change to 'if (!supply_names)' and omit the initialization of > > .supply_names for devices that use the generic supply names. > > > > That looks much cleaner, I will apply it to the next version. > > >> diff --git a/drivers/usb/misc/onboard_usb_dev.h > >> b/drivers/usb/misc/onboard_usb_dev.h > >> index ebe83e19d818..59dced6bd339 100644 > >> --- a/drivers/usb/misc/onboard_usb_dev.h > >> +++ b/drivers/usb/misc/onboard_usb_dev.h > >> @@ -6,63 +6,86 @@ > >> #ifndef _USB_MISC_ONBOARD_USB_DEV_H > >> #define _USB_MISC_ONBOARD_USB_DEV_H > >> > >> +/* > >> + * Fallback supply names for backwards compatibility. If the device > >> requires > >> + * more than the currently supported supplies, add a new one here, and if > >> + * possible, the real name supplies to the device-specific data. > >> + */ > >> +static const char * const generic_supply_names[] = { > >> + "vdd", > >> + "vdd2", > >> +}; > >> + > >> +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names) > > > > This will have to change when support for a device with more than 2 > > non-generic > > supply names gets added. Please use a literal value for MAX_SUPPLIES > > instead of > > ARRAY_SIZE. If the literal is 2 it would still need to change for future > > devices > > with more supplies, but that change would be more straighforward. > > > > I am not completely sure about this. Someone could increase MAX_SUPPLIES > without adding a generic name. That's perfectly fine and intended. MAX_SUPPLIES is a max, any list shorther than that is valid. Any longer list will result in probe() being aborted with a clear error message. > Actually two modifications will be necessary for every addition (name > and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required, > and MAX_SUPPLIES is automatically increased. As per above it's not necessary to add a new name when MAX_SUPPLIES is increased to support more non-generic names. It would only be necessary if more generic names were added, my understanding is that this should not happen because any newly supported onboard devices are supposed to use device specific supply names. I don't like to idea of adding unused pseudo supply names to the list, just for the sake of using ARRAY_SIZE. > I understand that the whole point of this is getting rid of the generic > names, but we still have to provide generic names for every extra > supply, at least for code consistency and to avoid size mismatches > between real an generic supply names. Please let me know if you still think the extra names are needed.
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Wed, Feb 21, 2024 at 11:06:10PM +0200, Andy Shevchenko wrote: > On Wed, Feb 21, 2024 at 10:37:30PM +0200, Dmitry Baryshkov wrote: > > On Wed, 21 Feb 2024 at 22:30, Dmitry Baryshkov > > wrote: ... > > Excuse me, it seems a c from gitlab didn't work as expected. > > No problem, it's clear that the patch has not been properly tested and > obviously wrong. Has to be dropped. If somebody wants that, it will > be material for v6.10 cycle. ...which makes me think that bitmap(bitops) lack of assembly test case(s). -- With Best Regards, Andy Shevchenko
Re: [PATCH 5/9] rtc: test: Fix invalid format specifier.
Hi, On Wed, Feb 21, 2024 at 05:27:18PM +0800, David Gow wrote: > 'days' is a s64 (from div_s64), and so should use a %lld specifier. > > This was found by extending KUnit's assertion macros to use gcc's > __printf attribute. > > Fixes: 1d1bb12a8b18 ("rtc: Improve performance of rtc_time64_to_tm(). Add > tests.") > Signed-off-by: David Gow Reviewed-by: Justin Stitt > --- > drivers/rtc/lib_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/rtc/lib_test.c b/drivers/rtc/lib_test.c > index d5caf36c56cd..225c859d6da5 100644 > --- a/drivers/rtc/lib_test.c > +++ b/drivers/rtc/lib_test.c > @@ -54,7 +54,7 @@ static void rtc_time64_to_tm_test_date_range(struct kunit > *test) > > days = div_s64(secs, 86400); > > - #define FAIL_MSG "%d/%02d/%02d (%2d) : %ld", \ > + #define FAIL_MSG "%d/%02d/%02d (%2d) : %lld", \ > year, month, mday, yday, days > > KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year, > FAIL_MSG); > -- > 2.44.0.rc0.258.g7320e95886-goog > Thanks Justin
Re: [PATCH 4/9] time: test: Fix incorrect format specifier
Hi, On Wed, Feb 21, 2024 at 05:27:17PM +0800, David Gow wrote: > 'days' is a s64 (from div_s64), and so should use a %lld specifier. > > This was found by extending KUnit's assertion macros to use gcc's > __printf attribute. > > Fixes: 276010551664 ("time: Improve performance of time64_to_tm()") > Signed-off-by: David Gow Reviewed-by: Justin Stitt > --- > kernel/time/time_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/time/time_test.c b/kernel/time/time_test.c > index ca058c8af6ba..3e5d422dd15c 100644 > --- a/kernel/time/time_test.c > +++ b/kernel/time/time_test.c > @@ -73,7 +73,7 @@ static void time64_to_tm_test_date_range(struct kunit *test) > > days = div_s64(secs, 86400); > > - #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %ld", \ > + #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %lld", \ > year, month, mdday, yday, days > > KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year, > FAIL_MSG); > -- > 2.44.0.rc0.258.g7320e95886-goog > Thanks Justin
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Wed, Feb 21, 2024 at 10:37:30PM +0200, Dmitry Baryshkov wrote: > On Wed, 21 Feb 2024 at 22:30, Dmitry Baryshkov > wrote: ... > Excuse me, it seems a c from gitlab didn't work as expected. No problem, it's clear that the patch has not been properly tested and obviously wrong. Has to be dropped. If somebody wants that, it will be material for v6.10 cycle. -- With Best Regards, Andy Shevchenko
Re: [PATCH 3/9] lib: memcpy_kunit: Fix an invalid format specifier in an assertion msg
Hi, On Wed, Feb 21, 2024 at 05:27:16PM +0800, David Gow wrote: > The 'i' passed as an assertion message is a size_t, so should use '%zu', > not '%d'. > > This was found by annotating the _MSG() variants of KUnit's assertions > to let gcc validate the format strings. > > Fixes: bb95ebbe89a7 ("lib: Introduce CONFIG_MEMCPY_KUNIT_TEST") > Signed-off-by: David Gow > --- Reviewed-by: Justin Stitt > lib/memcpy_kunit.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c > index 440aee705ccc..30e00ef0bf2e 100644 > --- a/lib/memcpy_kunit.c > +++ b/lib/memcpy_kunit.c > @@ -32,7 +32,7 @@ struct some_bytes { > BUILD_BUG_ON(sizeof(instance.data) != 32); \ > for (size_t i = 0; i < sizeof(instance.data); i++) {\ > KUNIT_ASSERT_EQ_MSG(test, instance.data[i], v, \ > - "line %d: '%s' not initialized to 0x%02x @ %d (saw > 0x%02x)\n", \ > + "line %d: '%s' not initialized to 0x%02x @ %zu (saw > 0x%02x)\n", \ > __LINE__, #instance, v, i, instance.data[i]); \ > } \ > } while (0) > @@ -41,7 +41,7 @@ struct some_bytes { > BUILD_BUG_ON(sizeof(one) != sizeof(two)); \ > for (size_t i = 0; i < sizeof(one); i++) { \ > KUNIT_EXPECT_EQ_MSG(test, one.data[i], two.data[i], \ > - "line %d: %s.data[%d] (0x%02x) != %s.data[%d] > (0x%02x)\n", \ > + "line %d: %s.data[%zu] (0x%02x) != %s.data[%zu] > (0x%02x)\n", \ > __LINE__, #one, i, one.data[i], #two, i, two.data[i]); \ > } \ > kunit_info(test, "ok: " TEST_OP "() " name "\n"); \ > -- > 2.44.0.rc0.258.g7320e95886-goog > Thanks Justin
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Wed, Feb 21, 2024 at 10:30:02PM +0200, Dmitry Baryshkov wrote: > On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi wrote: ... > > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) Can sizeof() be used in assembly? ... > > -#define __GENMASK(h, l) \ > > - (((~UL(0)) - (UL(1) << (l)) + 1) & \ > > -(~UL(0) >> (BITS_PER_LONG - 1 - (h > > -#define GENMASK(h, l) \ > > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)) > > +#define __GENMASK(t, h, l) \ > > + (GENMASK_INPUT_CHECK(h, l) + \ > > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \ > > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h) Nevertheless, the use ~0ULL is not proper assembly, this broke initial implementation using UL() / ULL(). > > -#define __GENMASK_ULL(h, l) \ > > - (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ > > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h > > -#define GENMASK_ULL(h, l) \ > > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l)) Ditto. > > +#define GENMASK(h, l) __GENMASK(unsigned long, h, l) > > +#define GENMASK_ULL(h, l) __GENMASK(unsigned long long, h, l) > > +#define GENMASK_U8(h, l) __GENMASK(u8, h, l) > > +#define GENMASK_U16(h, l) __GENMASK(u16, h, l) > > +#define GENMASK_U32(h, l) __GENMASK(u32, h, l) > > +#define GENMASK_U64(h, l) __GENMASK(u64, h, l) > > This breaks drm-tip on arm64 architecture: > > arch/arm64/kernel/entry-fpsimd.S: Assembler messages: > 465arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' > 466arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here > 467arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' > 468arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here > 469arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' > 470arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here > 471arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' > 472arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here > 473arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters > following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned > long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned > long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)' > 474arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here > 475arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' > 476arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here > 477arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' > 478arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here > 479arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' > 480arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here > 481arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' > 482arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here > 483arch/arm64/kernel/entry-fpsimd.S:282: Error: unexpected characters > following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned > long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned > long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)' > 484arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 2/2] drm/i915/gt: Enable only one CCS for compute workload
On Wed, Feb 21, 2024 at 01:12:18AM +0100, Andi Shyti wrote: > Hi Matt, > > thanks a lot for looking into this. > > On Tue, Feb 20, 2024 at 03:39:18PM -0800, Matt Roper wrote: > > On Tue, Feb 20, 2024 at 03:35:26PM +0100, Andi Shyti wrote: > > [...] > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > b/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > index 833987015b8b..7041acc77810 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c > > > @@ -243,6 +243,15 @@ void intel_engines_driver_register(struct > > > drm_i915_private *i915) > > > if (engine->uabi_class == I915_NO_UABI_CLASS) > > > continue; > > > > > > + /* > > > + * Do not list and do not count CCS engines other than the first > > > + */ > > > + if (engine->uabi_class == I915_ENGINE_CLASS_COMPUTE && > > > + engine->uabi_instance > 0) { > > > + i915->engine_uabi_class_count[engine->uabi_class]--; > > > + continue; > > > + } > > > > Wouldn't it be simpler to just add a workaround to the end of > > engine_mask_apply_compute_fuses() if we want to ensure only a single > > compute engine gets exposed? Then both the driver internals and uapi > > will agree that's there's just one CCS (and on which one there is). > > > > If we want to do something fancy with "hotplugging" a new engine later > > on or whatever, that can be handled in the future series (although as > > noted on the previous patch, it sounds like these changes might not > > actually be aligned with the workaround we were trying to address). > > The hotplugging capability is one of the features I was looking > for, actually. > > I have done some more refactoring in this piece of code in > upcoming patches. > > Will check, though, if I can do something with compute_fuses(), > even though, the other cslices are not really fused off (read > below). > > > > + > > > rb_link_node(>uabi_node, prev, p); > > > rb_insert_color(>uabi_node, >uabi_engines); > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c > > > b/drivers/gpu/drm/i915/gt/intel_gt.c > > > index a425db5ed3a2..e19df4ef47f6 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > > > @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt) > > > } > > > } > > > > > > +static void intel_gt_apply_ccs_mode(struct intel_gt *gt) > > > +{ > > > + if (!IS_DG2(gt->i915)) > > > + return; > > > + > > > + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0); > > > > This doesn't look right to me. A value of 0 means every cslice gets > > associated with CCS0. > > Yes, that's what I'm trying to do. The behavior I'm looking for > is this one: > >/* > ... > * With 1 engine (ccs0): > * slice 0, 1, 2, 3: ccs0 > * > * With 2 engines (ccs0, ccs1): > * slice 0, 2: ccs0 > * slice 1, 3: ccs1 > * > * With 4 engines (ccs0, ccs1, ccs2, ccs3): > * slice 0: ccs0 > * slice 1: ccs1 > * slice 2: ccs2 > * slice 3: ccs3 > ... > */ > > where the user can configure runtime the mode, making sure that > no client is connected to i915. > > But, this needs to be written > > As we are now forcing mode '1', then all cslices are connected > with ccs0. Right --- and that's what I'm pointing out as illegal. I think that code comment above was taken out of context from a different RFC series; that's not an accurate description of the behavior we want here. First, the above comment is using ccs# to refer to userspace engines, not hardware engines. As a simple example, DG2-G11 only ever has a single CCS which userspace sees as "instance 0" but which is actually CCS1 at the hardware level. If you try to follow the comment above when programming CCS_MODE, you've assigned all of the cslices to a non-existent engine and assigned no cslices to the CCS engine that actually exists. For DG2-G10 (and I think DG2-G12), there are different combinations of fused-off / not-fused-off engines that will always show up in userspace as CCS0-CCSn, even if those don't match the hardware IDs. Second, the above comment is assuming that you have a part with a maximum fusing config (i.e., all cslices present). Using DG2-G11 again as an example, there's also only a single cslice (cslice1), so if you tell CCS1 that it's allowed to use EUs from non-existent cslice0, cslice2, and cslice3, you might not get the behavior you were hoping for. > > > On a DG2-G11 platform, that will flat out break > > compute since CCS0 is never present (G11 only has a single CCS and it's > > always the hardware's CCS1). Even on a G10 or G12 this could also break > > things depending on the fusing of your card if the hardware CCS0 happens > >
Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names
On 21.02.24 21:25, Matthias Kaehlcke wrote: > On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote: >> The current mechanism uses generic names for the power supplies, which >> conflicts with proper name definitions in the device bindings. >> >> Add a per-device property to include real supply names and keep generic >> names as a fallback mechanism for backward compatibility. >> >> Signed-off-by: Javier Carrasco >> --- >> drivers/usb/misc/onboard_usb_dev.c | 54 >> -- >> drivers/usb/misc/onboard_usb_dev.h | 23 >> 2 files changed, 52 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/usb/misc/onboard_usb_dev.c >> b/drivers/usb/misc/onboard_usb_dev.c >> index f43130a6786f..e66fcac93006 100644 >> --- a/drivers/usb/misc/onboard_usb_dev.c >> +++ b/drivers/usb/misc/onboard_usb_dev.c >> @@ -29,18 +29,6 @@ >> >> #include "onboard_usb_dev.h" >> >> -/* >> - * Use generic names, as the actual names might differ between devices. If >> a new >> - * device requires more than the currently supported supplies, add a new one >> - * here. >> - */ >> -static const char * const supply_names[] = { >> -"vdd", >> -"vdd2", >> -}; >> - >> -#define MAX_SUPPLIES ARRAY_SIZE(supply_names) >> - >> static void onboard_dev_attach_usb_driver(struct work_struct *work); >> >> static struct usb_device_driver onboard_dev_usbdev_driver; >> @@ -66,6 +54,33 @@ struct onboard_dev { >> struct clk *clk; >> }; >> >> +static int onboard_dev_get_regulator_bulk(struct device *dev, >> + struct onboard_dev *onboard_dev) >> +{ >> +unsigned int i; >> +int err; >> + >> +const char * const *supply_names = onboard_dev->pdata->supply_names; >> + >> +if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES) >> +return dev_err_probe(dev, -EINVAL, "max %zu supplies >> supported!\n", >> + MAX_SUPPLIES); >> + >> +if (!supply_names[0]) >> +supply_names = generic_supply_names; > > Please change to 'if (!supply_names)' and omit the initialization of > .supply_names for devices that use the generic supply names. > That looks much cleaner, I will apply it to the next version. >> diff --git a/drivers/usb/misc/onboard_usb_dev.h >> b/drivers/usb/misc/onboard_usb_dev.h >> index ebe83e19d818..59dced6bd339 100644 >> --- a/drivers/usb/misc/onboard_usb_dev.h >> +++ b/drivers/usb/misc/onboard_usb_dev.h >> @@ -6,63 +6,86 @@ >> #ifndef _USB_MISC_ONBOARD_USB_DEV_H >> #define _USB_MISC_ONBOARD_USB_DEV_H >> >> +/* >> + * Fallback supply names for backwards compatibility. If the device requires >> + * more than the currently supported supplies, add a new one here, and if >> + * possible, the real name supplies to the device-specific data. >> + */ >> +static const char * const generic_supply_names[] = { >> +"vdd", >> +"vdd2", >> +}; >> + >> +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names) > > This will have to change when support for a device with more than 2 > non-generic > supply names gets added. Please use a literal value for MAX_SUPPLIES instead > of > ARRAY_SIZE. If the literal is 2 it would still need to change for future > devices > with more supplies, but that change would be more straighforward. > I am not completely sure about this. Someone could increase MAX_SUPPLIES without adding a generic name. Actually two modifications will be necessary for every addition (name and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required, and MAX_SUPPLIES is automatically increased. I understand that the whole point of this is getting rid of the generic names, but we still have to provide generic names for every extra supply, at least for code consistency and to avoid size mismatches between real an generic supply names. >> + >> struct onboard_dev_pdata { >> unsigned long reset_us; /* reset pulse width in us */ >> unsigned int num_supplies; /* number of supplies */ >> bool is_hub; >> +const char * const supply_names[MAX_SUPPLIES]; >> + >> }; >> >> static const struct onboard_dev_pdata microchip_usb424_data = { >> .reset_us = 1, >> .num_supplies = 1, >> +.supply_names = { NULL }, >> .is_hub = true, >> }; >> >> ... Thanks again for your feedback. Best regards, Javier Carrasco
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Wed, 21 Feb 2024 at 22:30, Dmitry Baryshkov wrote: > > On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi wrote: > > > > From: Yury Norov > > > > Generalize __GENMASK() to support different types, and implement > > fixed-types versions of GENMASK() based on it. The fixed-type version > > allows more strict checks to the min/max values accepted, which is > > useful for defining registers like implemented by i915 and xe drivers > > with their REG_GENMASK*() macros. > > > > The strict checks rely on shift-count-overflow compiler check to > > fail the build if a number outside of the range allowed is passed. > > Example: > > > > #define FOO_MASK GENMASK_U32(33, 4) > > > > will generate a warning like: > > > > ../include/linux/bits.h:41:31: error: left shift count >= width of > > type [-Werror=shift-count-overflow] > >41 | (((t)~0ULL - ((t)(1) << (l)) + 1) & \ > > | ^~ > > > > Signed-off-by: Yury Norov > > Signed-off-by: Lucas De Marchi > > Acked-by: Jani Nikula > > --- > > include/linux/bitops.h | 1 - > > include/linux/bits.h | 32 ++-- > > 2 files changed, 22 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > > index 2ba557e067fe..1db50c69cfdb 100644 > > --- a/include/linux/bitops.h > > +++ b/include/linux/bitops.h > > @@ -15,7 +15,6 @@ > > # define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n))) > > #endif > > > > -#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > > #define BITS_TO_LONGS(nr) __KERNEL_DIV_ROUND_UP(nr, > > BITS_PER_TYPE(long)) > > #define BITS_TO_U64(nr)__KERNEL_DIV_ROUND_UP(nr, > > BITS_PER_TYPE(u64)) > > #define BITS_TO_U32(nr)__KERNEL_DIV_ROUND_UP(nr, > > BITS_PER_TYPE(u32)) > > diff --git a/include/linux/bits.h b/include/linux/bits.h > > index 7c0cf5031abe..bd56f32de44e 100644 > > --- a/include/linux/bits.h > > +++ b/include/linux/bits.h > > @@ -6,6 +6,8 @@ > > #include > > #include > > > > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > > + > > #define BIT_MASK(nr) (UL(1) << ((nr) % BITS_PER_LONG)) > > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > > #define BIT_ULL_MASK(nr) (ULL(1) << ((nr) % BITS_PER_LONG_LONG)) > > @@ -30,16 +32,26 @@ > > #define GENMASK_INPUT_CHECK(h, l) 0 > > #endif > > > > -#define __GENMASK(h, l) \ > > - (((~UL(0)) - (UL(1) << (l)) + 1) & \ > > -(~UL(0) >> (BITS_PER_LONG - 1 - (h > > -#define GENMASK(h, l) \ > > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)) > > +/* > > + * Generate a mask for the specified type @t. Additional checks are made to > > + * guarantee the value returned fits in that type, relying on > > + * shift-count-overflow compiler check to detect incompatible arguments. > > + * For example, all these create build errors or warnings: > > + * > > + * - GENMASK(15, 20): wrong argument order > > + * - GENMASK(72, 15): doesn't fit unsigned long > > + * - GENMASK_U32(33, 15): doesn't fit in a u32 > > + */ > > +#define __GENMASK(t, h, l) \ > > + (GENMASK_INPUT_CHECK(h, l) + \ > > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \ > > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h) > > > > -#define __GENMASK_ULL(h, l) \ > > - (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ > > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h > > -#define GENMASK_ULL(h, l) \ > > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l)) > > +#define GENMASK(h, l) __GENMASK(unsigned long, h, l) > > +#define GENMASK_ULL(h, l) __GENMASK(unsigned long long, h, l) > > +#define GENMASK_U8(h, l) __GENMASK(u8, h, l) > > +#define GENMASK_U16(h, l) __GENMASK(u16, h, l) > > +#define GENMASK_U32(h, l) __GENMASK(u32, h, l) > > +#define GENMASK_U64(h, l) __GENMASK(u64, h, l) > > This breaks drm-tip on arm64 architecture: > Excuse me, it seems a c from gitlab didn't work as expected. arch/arm64/kernel/entry-fpsimd.S: Assembler messages: arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)' arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l',
Re: [PATCH] drm/amd/display: Add prefix to functions inside dnc10_cm_common.h
On Wed, Feb 21, 2024 at 2:31 PM Joao Paulo Pereira da Silva wrote: > > While debugging with ftrace, it's useful to create filters to search > within the codebase. However, since some function names lack prefixes, > creating a good filter may become more difficult. > > Because of this, add prefix to functions declared inside the header > dcn10/dcn10_cm_common.h to facilitate creating good filters for the > functions declared inside the file. > I think in this case, these functions are not dcn10 specific, they are common to all DCN families, hence, the lack of a dcn10 prefix, they just happen to date back to the original dcn10 code. Might be better to split them out into a separate file rather than renaming them. Alex > Signed-off-by: Joao Paulo Pereira da Silva > --- > .../drm/amd/display/dc/dcn10/dcn10_cm_common.c | 12 ++-- > .../drm/amd/display/dc/dcn10/dcn10_cm_common.h | 8 > .../drm/amd/display/dc/dcn10/dcn10_dpp_cm.c| 18 +- > .../drm/amd/display/dc/dcn20/dcn20_dpp_cm.c| 8 > .../gpu/drm/amd/display/dc/dcn20/dcn20_mpc.c | 8 > .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 2 +- > .../drm/amd/display/dc/dcn30/dcn30_dpp_cm.c| 4 ++-- > .../drm/amd/display/dc/dcn30/dcn30_dwb_cm.c| 6 +++--- > .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 8 > .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c| 2 +- > .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c| 6 +++--- > .../amd/display/dc/hwss/dcn30/dcn30_hwseq.c| 2 +- > .../amd/display/dc/hwss/dcn32/dcn32_hwseq.c| 2 +- > 13 files changed, 43 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c > index 3538973bd0c6..3878b78faf89 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c > @@ -38,7 +38,7 @@ > #define FN(reg_name, field_name) \ > reg->shifts.field_name, reg->masks.field_name > > -void cm_helper_program_color_matrices( > +void dcn10_cm_common_helper_program_color_matrices( > struct dc_context *ctx, > const uint16_t *regval, > const struct color_matrices_reg *reg) > @@ -62,7 +62,7 @@ void cm_helper_program_color_matrices( > > } > > -void cm_helper_program_xfer_func( > +void dcn10_cm_common_helper_program_xfer_func( > struct dc_context *ctx, > const struct pwl_params *params, > const struct xfer_func_reg *reg) > @@ -125,7 +125,7 @@ void cm_helper_program_xfer_func( > > > > -bool cm_helper_convert_to_custom_float( > +bool dcn10_cm_common_helper_convert_to_custom_float( > struct pwl_result_data *rgb_resulted, > struct curve_points3 *corner_points, > uint32_t hw_points_num, > @@ -311,7 +311,7 @@ bool cm_helper_convert_to_custom_float( > #define DC_LOGGER \ > ctx->logger > > -bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx, > +bool dcn10_cm_common_helper_translate_curve_to_hw_format(struct dc_context > *ctx, > const struct dc_transfer_func *output_tf, > struct pwl_params *lut_params, bool fixpoint) > { > @@ -507,7 +507,7 @@ bool cm_helper_translate_curve_to_hw_format(struct > dc_context *ctx, > ++rgb; > ++i; > } > - cm_helper_convert_to_custom_float(rgb_resulted, > + dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted, > lut_params->corner_points, > hw_points, fixpoint); > > @@ -653,7 +653,7 @@ bool cm_helper_translate_curve_to_degamma_hw_format( > ++rgb; > ++i; > } > - cm_helper_convert_to_custom_float(rgb_resulted, > + dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted, > lut_params->corner_points, > hw_points, false); > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h > index 0a68b63d6126..0622dbdbe84b 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h > @@ -89,23 +89,23 @@ struct color_matrices_reg{ > uint32_t csc_c33_c34; > }; > > -void cm_helper_program_color_matrices( > +void dcn10_cm_common_helper_program_color_matrices( > struct dc_context *ctx, > const uint16_t *regval, > const struct color_matrices_reg *reg); > > -void cm_helper_program_xfer_func( > +void dcn10_cm_common_helper_program_xfer_func( > struct dc_context *ctx, > const struct pwl_params
Re: [PATCH 0/3] drm/amdgpu: Use KMEM_CACHE instead of kmem_cache_create
Applied. Thanks! On Wed, Feb 21, 2024 at 6:08 AM Christian König wrote: > > Am 21.02.24 um 10:59 schrieb Kunwu Chan: > > For where the cache name and the structure name match. > > Use the new KMEM_CACHE() macro instead of direct kmem_cache_create > > to simplify the creation of SLAB caches. > > Reviewed-by: Christian König for the entire > series. > > > > > Kunwu Chan (3): > >drm/amdgpu: Simplify the allocation of fence slab caches > >drm/amdgpu: Simplify the allocation of mux_chunk slab caches > >drm/amdgpu: Simplify the allocation of sync slab caches > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c| 4 +--- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c | 4 +--- > > drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 4 +--- > > 3 files changed, 3 insertions(+), 9 deletions(-) > > >
Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks
On Thu, 8 Feb 2024 at 09:45, Lucas De Marchi wrote: > > From: Yury Norov > > Generalize __GENMASK() to support different types, and implement > fixed-types versions of GENMASK() based on it. The fixed-type version > allows more strict checks to the min/max values accepted, which is > useful for defining registers like implemented by i915 and xe drivers > with their REG_GENMASK*() macros. > > The strict checks rely on shift-count-overflow compiler check to > fail the build if a number outside of the range allowed is passed. > Example: > > #define FOO_MASK GENMASK_U32(33, 4) > > will generate a warning like: > > ../include/linux/bits.h:41:31: error: left shift count >= width of > type [-Werror=shift-count-overflow] >41 | (((t)~0ULL - ((t)(1) << (l)) + 1) & \ > | ^~ > > Signed-off-by: Yury Norov > Signed-off-by: Lucas De Marchi > Acked-by: Jani Nikula > --- > include/linux/bitops.h | 1 - > include/linux/bits.h | 32 ++-- > 2 files changed, 22 insertions(+), 11 deletions(-) > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index 2ba557e067fe..1db50c69cfdb 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -15,7 +15,6 @@ > # define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n))) > #endif > > -#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > #define BITS_TO_LONGS(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(long)) > #define BITS_TO_U64(nr)__KERNEL_DIV_ROUND_UP(nr, > BITS_PER_TYPE(u64)) > #define BITS_TO_U32(nr)__KERNEL_DIV_ROUND_UP(nr, > BITS_PER_TYPE(u32)) > diff --git a/include/linux/bits.h b/include/linux/bits.h > index 7c0cf5031abe..bd56f32de44e 100644 > --- a/include/linux/bits.h > +++ b/include/linux/bits.h > @@ -6,6 +6,8 @@ > #include > #include > > +#define BITS_PER_TYPE(type)(sizeof(type) * BITS_PER_BYTE) > + > #define BIT_MASK(nr) (UL(1) << ((nr) % BITS_PER_LONG)) > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > #define BIT_ULL_MASK(nr) (ULL(1) << ((nr) % BITS_PER_LONG_LONG)) > @@ -30,16 +32,26 @@ > #define GENMASK_INPUT_CHECK(h, l) 0 > #endif > > -#define __GENMASK(h, l) \ > - (((~UL(0)) - (UL(1) << (l)) + 1) & \ > -(~UL(0) >> (BITS_PER_LONG - 1 - (h > -#define GENMASK(h, l) \ > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)) > +/* > + * Generate a mask for the specified type @t. Additional checks are made to > + * guarantee the value returned fits in that type, relying on > + * shift-count-overflow compiler check to detect incompatible arguments. > + * For example, all these create build errors or warnings: > + * > + * - GENMASK(15, 20): wrong argument order > + * - GENMASK(72, 15): doesn't fit unsigned long > + * - GENMASK_U32(33, 15): doesn't fit in a u32 > + */ > +#define __GENMASK(t, h, l) \ > + (GENMASK_INPUT_CHECK(h, l) + \ > +(((t)~0ULL - ((t)(1) << (l)) + 1) & \ > +((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h) > > -#define __GENMASK_ULL(h, l) \ > - (((~ULL(0)) - (ULL(1) << (l)) + 1) & \ > -(~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h > -#define GENMASK_ULL(h, l) \ > - (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l)) > +#define GENMASK(h, l) __GENMASK(unsigned long, h, l) > +#define GENMASK_ULL(h, l) __GENMASK(unsigned long long, h, l) > +#define GENMASK_U8(h, l) __GENMASK(u8, h, l) > +#define GENMASK_U16(h, l) __GENMASK(u16, h, l) > +#define GENMASK_U32(h, l) __GENMASK(u32, h, l) > +#define GENMASK_U64(h, l) __GENMASK(u64, h, l) This breaks drm-tip on arm64 architecture: arch/arm64/kernel/entry-fpsimd.S: Assembler messages: 465arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 466arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 467arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 468arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 469arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 470arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 471arch/arm64/kernel/entry-fpsimd.S:271: Error: found 'l', expected: ')' 472arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 473arch/arm64/kernel/entry-fpsimd.S:271: Error: unexpected characters following instruction at operand 3 -- `bic x2,x1,(0+(((unsigned long)~0ULL-((unsigned long)(1)<<(0))+1)&((unsigned long)~0ULL>>((sizeof(unsigned long)*8)-1-(3)' 474arch/arm64/kernel/entry-fpsimd.S:66: Info: macro invoked from here 475arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 476arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 477arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l', expected: ')' 478arch/arm64/kernel/entry-fpsimd.S:98: Info: macro invoked from here 479arch/arm64/kernel/entry-fpsimd.S:282: Error: found 'l',
Re: [PATCH 1/9] kunit: test: Log the correct filter string in executor_test
On Wed, Feb 21, 2024 at 1:28 AM David Gow wrote: > > KUnit's executor_test logs the filter string in KUNIT_ASSERT_EQ_MSG(), > but passed a random character from the filter, rather than the whole > string. Note: it's worse than that, afaict. It's printing from a random bit of memory. I was curious about this, so I found under UML, the string I got was always "efault)" if I make it fail for j=0. > > This was found by annotating KUNIT_ASSERT_EQ_MSG() to let gcc validate > the format string. > > Fixes: 76066f93f1df ("kunit: add tests for filtering attributes") > Signed-off-by: David Gow Reviewed-by: Daniel Latypov > --- > lib/kunit/executor_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c > index 22d4ee86dbed..3f7f967e3688 100644 > --- a/lib/kunit/executor_test.c > +++ b/lib/kunit/executor_test.c > @@ -129,7 +129,7 @@ static void parse_filter_attr_test(struct kunit *test) > GFP_KERNEL); > for (j = 0; j < filter_count; j++) { > parsed_filters[j] = kunit_next_attr_filter(, ); > - KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter > '%s'", filters[j]); > + KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter > from '%s'", filters); note: if there is a v2, it might be nice to include `j` in the message.
Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names
On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote: > The current mechanism uses generic names for the power supplies, which > conflicts with proper name definitions in the device bindings. > > Add a per-device property to include real supply names and keep generic > names as a fallback mechanism for backward compatibility. > > Signed-off-by: Javier Carrasco > --- > drivers/usb/misc/onboard_usb_dev.c | 54 > -- > drivers/usb/misc/onboard_usb_dev.h | 23 > 2 files changed, 52 insertions(+), 25 deletions(-) > > diff --git a/drivers/usb/misc/onboard_usb_dev.c > b/drivers/usb/misc/onboard_usb_dev.c > index f43130a6786f..e66fcac93006 100644 > --- a/drivers/usb/misc/onboard_usb_dev.c > +++ b/drivers/usb/misc/onboard_usb_dev.c > @@ -29,18 +29,6 @@ > > #include "onboard_usb_dev.h" > > -/* > - * Use generic names, as the actual names might differ between devices. If a > new > - * device requires more than the currently supported supplies, add a new one > - * here. > - */ > -static const char * const supply_names[] = { > - "vdd", > - "vdd2", > -}; > - > -#define MAX_SUPPLIES ARRAY_SIZE(supply_names) > - > static void onboard_dev_attach_usb_driver(struct work_struct *work); > > static struct usb_device_driver onboard_dev_usbdev_driver; > @@ -66,6 +54,33 @@ struct onboard_dev { > struct clk *clk; > }; > > +static int onboard_dev_get_regulator_bulk(struct device *dev, > + struct onboard_dev *onboard_dev) > +{ > + unsigned int i; > + int err; > + > + const char * const *supply_names = onboard_dev->pdata->supply_names; > + > + if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES) > + return dev_err_probe(dev, -EINVAL, "max %zu supplies > supported!\n", > + MAX_SUPPLIES); > + > + if (!supply_names[0]) > + supply_names = generic_supply_names; Please change to 'if (!supply_names)' and omit the initialization of .supply_names for devices that use the generic supply names. > diff --git a/drivers/usb/misc/onboard_usb_dev.h > b/drivers/usb/misc/onboard_usb_dev.h > index ebe83e19d818..59dced6bd339 100644 > --- a/drivers/usb/misc/onboard_usb_dev.h > +++ b/drivers/usb/misc/onboard_usb_dev.h > @@ -6,63 +6,86 @@ > #ifndef _USB_MISC_ONBOARD_USB_DEV_H > #define _USB_MISC_ONBOARD_USB_DEV_H > > +/* > + * Fallback supply names for backwards compatibility. If the device requires > + * more than the currently supported supplies, add a new one here, and if > + * possible, the real name supplies to the device-specific data. > + */ > +static const char * const generic_supply_names[] = { > + "vdd", > + "vdd2", > +}; > + > +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names) This will have to change when support for a device with more than 2 non-generic supply names gets added. Please use a literal value for MAX_SUPPLIES instead of ARRAY_SIZE. If the literal is 2 it would still need to change for future devices with more supplies, but that change would be more straighforward. > + > struct onboard_dev_pdata { > unsigned long reset_us; /* reset pulse width in us */ > unsigned int num_supplies; /* number of supplies */ > bool is_hub; > + const char * const supply_names[MAX_SUPPLIES]; > + > }; > > static const struct onboard_dev_pdata microchip_usb424_data = { > .reset_us = 1, > .num_supplies = 1, > + .supply_names = { NULL }, > .is_hub = true, > }; > > ...
Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg
Hi, On Wed, Feb 21, 2024 at 05:27:15PM +0800, David Gow wrote: > The correct format specifier for p - n (both p and n are pointers) is > %td, as the type should be ptrdiff_t. I think %tu is better. d specifies a signed type. I don't doubt that the warning is fixed but I think %tu represents the type semantics here. > > This was discovered by annotating KUnit assertion macros with gcc's > printf specifier, but note that gcc incorrectly suggested a %d or %ld > specifier (depending on the pointer size of the architecture being > built). > > Fixes: 0ea09083116d ("lib/cmdline: Allow get_options() to take 0 to validate > the input") > Signed-off-by: David Gow > --- > lib/cmdline_kunit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c > index d4572dbc9145..705b82736be0 100644 > --- a/lib/cmdline_kunit.c > +++ b/lib/cmdline_kunit.c > @@ -124,7 +124,7 @@ static void cmdline_do_one_range_test(struct kunit *test, > const char *in, > n, e[0], r[0]); > > p = memchr_inv([1], 0, sizeof(r) - sizeof(r[0])); > - KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %u out of bound", > n, p - r); > + KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %td out of > bound", n, p - r); > } > > static void cmdline_test_range(struct kunit *test) > -- > 2.44.0.rc0.258.g7320e95886-goog > Thanks Justin
Re: [PATCH 1/9] kunit: test: Log the correct filter string in executor_test
Hi, On Wed, Feb 21, 2024 at 05:27:14PM +0800, David Gow wrote: > KUnit's executor_test logs the filter string in KUNIT_ASSERT_EQ_MSG(), > but passed a random character from the filter, rather than the whole > string. > > This was found by annotating KUNIT_ASSERT_EQ_MSG() to let gcc validate > the format string. > > Fixes: 76066f93f1df ("kunit: add tests for filtering attributes") > Signed-off-by: David Gow Reviewed-by: Justin Stitt > --- > lib/kunit/executor_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c > index 22d4ee86dbed..3f7f967e3688 100644 > --- a/lib/kunit/executor_test.c > +++ b/lib/kunit/executor_test.c > @@ -129,7 +129,7 @@ static void parse_filter_attr_test(struct kunit *test) > GFP_KERNEL); > for (j = 0; j < filter_count; j++) { > parsed_filters[j] = kunit_next_attr_filter(, ); > - KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter > '%s'", filters[j]); > + KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter from > '%s'", filters); > } > > KUNIT_EXPECT_STREQ(test, kunit_attr_filter_name(parsed_filters[0]), > "speed"); > -- > 2.44.0.rc0.258.g7320e95886-goog > Thanks Justin
Re: [PATCH 9/9] kunit: Annotate _MSG assertion variants with gnu printf specifiers
Hi, On Wed, Feb 21, 2024 at 05:27:22PM +0800, David Gow wrote: > KUnit's assertion macros have variants which accept a printf format > string, to allow tests to specify a more detailed message on failure. > These (and the related KUNIT_FAIL() macro) ultimately wrap the > __kunit_do_failed_assertion() function, which accepted a printf format > specifier, but did not have the __printf attribute, so gcc couldn't warn > on incorrect agruments. > > It turns out there were quite a few tests with such incorrect arguments. > > Add the __printf() specifier now that we've fixed these errors, to > prevent them from recurring. > > Suggested-by: Linus Torvalds > Signed-off-by: David Gow Reviewed-by: Justin Stitt > --- > include/kunit/test.h | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/include/kunit/test.h b/include/kunit/test.h > index fcb4a4940ace..61637ef32302 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -579,12 +579,12 @@ void __printf(2, 3) kunit_log_append(struct > string_stream *log, const char *fmt, > > void __noreturn __kunit_abort(struct kunit *test); > > -void __kunit_do_failed_assertion(struct kunit *test, > -const struct kunit_loc *loc, > -enum kunit_assert_type type, > -const struct kunit_assert *assert, > -assert_format_t assert_format, > -const char *fmt, ...); > +void __printf(6, 7) __kunit_do_failed_assertion(struct kunit *test, > + const struct kunit_loc *loc, > + enum kunit_assert_type type, > + const struct kunit_assert > *assert, > + assert_format_t assert_format, > + const char *fmt, ...); > > #define _KUNIT_FAILED(test, assert_type, assert_class, assert_format, > INITIALIZER, fmt, ...) do { \ > static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \ > -- > 2.44.0.rc0.258.g7320e95886-goog > Thanks Justin
Re: Re: [PATCH] drm/xe: skip building debugfs code for CONFIG_DEBUG_FS=n
On Wed, Feb 21, 2024 at 05:21:17PM +1100, Stephen Rothwell wrote: Hi Lucas, On Tue, 20 Feb 2024 23:29:54 -0600 Lucas De Marchi wrote: Looking at https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2d5c7b7eb345249cb34d42cbc2b97b4c57ea944e it seems we still don't have drm-xe-next branch in linux-next. Stephen, could you please add it? Or do I have to do something on my end before that? This is the branch: https://gitlab.freedesktop.org/drm/xe/kernel drm-xe-next Added from tomorrow. Currently the only contact is yourself. Do you want anyone else (or a mailing list) as well? Do you want the drm-xe-fixes branch included as well? I don't think it's needed since drm-xe-next covers what is in drm-xe-fixes. Please add other maintainers and mailing list: M: Oded Gabbay M: Thomas Hellström L: intel...@lists.freedesktop.org Looking at drm-intel and drm-misc, they are using a special for-linux-next. We may eventually adopt the same workflow, but I will have to check with other maintainers. thanks Lucas De Marchi Thanks for adding your subsystem tree as a participant of linux-next. As you may know, this is not a judgement of your code. The purpose of linux-next is for integration testing and to lower the impact of conflicts between subsystems in the next merge window. You will need to ensure that the patches/commits in your tree/series have been: * submitted under GPL v2 (or later) and include the Contributor's Signed-off-by, * posted to the relevant mailing list, * reviewed by you (or another maintainer of your subsystem tree), * successfully unit tested, and * destined for the current or next Linux merge window. Basically, this should be just what you would send to Linus (or ask him to fetch). It is allowed to be rebased if you deem it necessary. -- Cheers, Stephen Rothwell s...@canb.auug.org.au
[PATCH] drm/amd/display: Add prefix to functions inside dnc10_cm_common.h
While debugging with ftrace, it's useful to create filters to search within the codebase. However, since some function names lack prefixes, creating a good filter may become more difficult. Because of this, add prefix to functions declared inside the header dcn10/dcn10_cm_common.h to facilitate creating good filters for the functions declared inside the file. Signed-off-by: Joao Paulo Pereira da Silva --- .../drm/amd/display/dc/dcn10/dcn10_cm_common.c | 12 ++-- .../drm/amd/display/dc/dcn10/dcn10_cm_common.h | 8 .../drm/amd/display/dc/dcn10/dcn10_dpp_cm.c| 18 +- .../drm/amd/display/dc/dcn20/dcn20_dpp_cm.c| 8 .../gpu/drm/amd/display/dc/dcn20/dcn20_mpc.c | 8 .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 2 +- .../drm/amd/display/dc/dcn30/dcn30_dpp_cm.c| 4 ++-- .../drm/amd/display/dc/dcn30/dcn30_dwb_cm.c| 6 +++--- .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 8 .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c| 2 +- .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c| 6 +++--- .../amd/display/dc/hwss/dcn30/dcn30_hwseq.c| 2 +- .../amd/display/dc/hwss/dcn32/dcn32_hwseq.c| 2 +- 13 files changed, 43 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c index 3538973bd0c6..3878b78faf89 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c @@ -38,7 +38,7 @@ #define FN(reg_name, field_name) \ reg->shifts.field_name, reg->masks.field_name -void cm_helper_program_color_matrices( +void dcn10_cm_common_helper_program_color_matrices( struct dc_context *ctx, const uint16_t *regval, const struct color_matrices_reg *reg) @@ -62,7 +62,7 @@ void cm_helper_program_color_matrices( } -void cm_helper_program_xfer_func( +void dcn10_cm_common_helper_program_xfer_func( struct dc_context *ctx, const struct pwl_params *params, const struct xfer_func_reg *reg) @@ -125,7 +125,7 @@ void cm_helper_program_xfer_func( -bool cm_helper_convert_to_custom_float( +bool dcn10_cm_common_helper_convert_to_custom_float( struct pwl_result_data *rgb_resulted, struct curve_points3 *corner_points, uint32_t hw_points_num, @@ -311,7 +311,7 @@ bool cm_helper_convert_to_custom_float( #define DC_LOGGER \ ctx->logger -bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx, +bool dcn10_cm_common_helper_translate_curve_to_hw_format(struct dc_context *ctx, const struct dc_transfer_func *output_tf, struct pwl_params *lut_params, bool fixpoint) { @@ -507,7 +507,7 @@ bool cm_helper_translate_curve_to_hw_format(struct dc_context *ctx, ++rgb; ++i; } - cm_helper_convert_to_custom_float(rgb_resulted, + dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted, lut_params->corner_points, hw_points, fixpoint); @@ -653,7 +653,7 @@ bool cm_helper_translate_curve_to_degamma_hw_format( ++rgb; ++i; } - cm_helper_convert_to_custom_float(rgb_resulted, + dcn10_cm_common_helper_convert_to_custom_float(rgb_resulted, lut_params->corner_points, hw_points, false); diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h index 0a68b63d6126..0622dbdbe84b 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.h @@ -89,23 +89,23 @@ struct color_matrices_reg{ uint32_t csc_c33_c34; }; -void cm_helper_program_color_matrices( +void dcn10_cm_common_helper_program_color_matrices( struct dc_context *ctx, const uint16_t *regval, const struct color_matrices_reg *reg); -void cm_helper_program_xfer_func( +void dcn10_cm_common_helper_program_xfer_func( struct dc_context *ctx, const struct pwl_params *params, const struct xfer_func_reg *reg); -bool cm_helper_convert_to_custom_float( +bool dcn10_cm_common_helper_convert_to_custom_float( struct pwl_result_data *rgb_resulted, struct curve_points3 *corner_points, uint32_t hw_points_num, bool fixpoint); -bool cm_helper_translate_curve_to_hw_format( +bool dcn10_cm_common_helper_translate_curve_to_hw_format( struct dc_context *ctx, const struct dc_transfer_func *output_tf,
Re: [PATCH v4 2/8] usb: misc: onboard_dev: add support for non-hub devices
On Tue, Feb 20, 2024 at 03:05:46PM +0100, Javier Carrasco wrote: > Most of the functionality this driver provides can be used by non-hub > devices as well. > > To account for the hub-specific code, add a flag to the device data > structure and check its value for hub-specific code. Please mention that the driver doesn't power off non-hub devices during system suspend. > Signed-off-by: Javier Carrasco > --- > drivers/usb/misc/onboard_usb_dev.c | 3 ++- > drivers/usb/misc/onboard_usb_dev.h | 10 ++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/onboard_usb_dev.c > b/drivers/usb/misc/onboard_usb_dev.c > index 2103af2cb2a6..f43130a6786f 100644 > --- a/drivers/usb/misc/onboard_usb_dev.c > +++ b/drivers/usb/misc/onboard_usb_dev.c > @@ -129,7 +129,8 @@ static int __maybe_unused onboard_dev_suspend(struct > device *dev) > if (!device_may_wakeup(node->udev->bus->controller)) > continue; > > - if (usb_wakeup_enabled_descendants(node->udev)) { > + if (usb_wakeup_enabled_descendants(node->udev) || > + !onboard_dev->pdata->is_hub) { This check isn't dependent on characteristics of the USB devices processed in this loop, therefore it can be performed at function entry. Please combine it with the check of 'always_powered_in_suspend'. It's also an option to omit the check completely, 'always_powered_in_suspend' will never be set for non-hub devices (assuming the sysfs attribute isn't added). > power_off = false; > break; > } Without code context: please omit the creation of the 'always_powered_in_suspend' attribute for non-hub devices. As per above we don't plan to hone it, so it shouldn't exist.
Re: [PATCH v4 1/8] usb: misc: onboard_hub: rename to onboard_dev
On Tue, Feb 20, 2024 at 03:05:45PM +0100, Javier Carrasco wrote: > This patch prepares onboad_hub to support non-hub devices by renaming > the driver files and their content, the headers and their references. > > The comments and descriptions have been slightly modified to keep > coherence and account for the specific cases that only affect onboard > hubs (e.g. peer-hub). > > The "hub" variables in functions where "dev" (and similar names) variables > already exist have been renamed to onboard_dev for clarity, which adds a > few lines in cases where more than 80 characters are used. > > No new functionality has been added. > > Signed-off-by: Javier Carrasco > --- > ...-usb-hub => sysfs-bus-platform-onboard-usb-dev} | 4 +- > MAINTAINERS| 4 +- > drivers/usb/core/Makefile | 4 +- > drivers/usb/core/hub.c | 8 +- > drivers/usb/core/hub.h | 2 +- > drivers/usb/misc/Kconfig | 16 +- > drivers/usb/misc/Makefile | 2 +- > drivers/usb/misc/onboard_usb_dev.c | 518 > + > .../misc/{onboard_usb_hub.h => onboard_usb_dev.h} | 28 +- > ...ard_usb_hub_pdevs.c => onboard_usb_dev_pdevs.c} | 47 +- > drivers/usb/misc/onboard_usb_hub.c | 501 > include/linux/usb/onboard_dev.h| 18 + > include/linux/usb/onboard_hub.h| 18 - > 13 files changed, 594 insertions(+), 576 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub > b/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev > similarity index 67% > rename from Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub > rename to Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev > index 42deb0552065..cd31f76362e7 100644 > --- a/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub > +++ b/Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-dev > @@ -4,5 +4,5 @@ KernelVersion:5.20 > Contact: Matthias Kaehlcke > linux-...@vger.kernel.org > Description: > - (RW) Controls whether the USB hub remains always powered > - during system suspend or not. > \ No newline at end of file > + (RW) Controls whether the USB device remains always powered > + during system suspend or not. With patch "[2/8] usb: misc: onboard_dev: add support for non-hub devices" this attribute isn't honed for non-hub devices. With that I'd say leave the existing comment unchanged, but add a note that this attribute only exists for hubs. That will also require a change in the patch mentioned above to omit the creation of the attribute for non-hub devices. > diff --git a/drivers/usb/misc/onboard_usb_dev.c > b/drivers/usb/misc/onboard_usb_dev.c > new file mode 100644 > index ..2103af2cb2a6 > --- /dev/null > +++ b/drivers/usb/misc/onboard_usb_dev.c > > ... > > +/* > + * Returns the onboard_dev platform device that is associated with the USB > + * device passed as parameter. > + */ > +static struct onboard_dev *_find_onboard_dev(struct device *dev) > +{ > + struct platform_device *pdev; > + struct device_node *np; > + struct onboard_dev *onboard_dev; > + > + pdev = of_find_device_by_node(dev->of_node); > + if (!pdev) { > + np = of_parse_phandle(dev->of_node, "peer-hub", 0); > + if (!np) { > + dev_err(dev, "failed to find device node for peer > hub\n"); > + return ERR_PTR(-EINVAL); > + } > + > + pdev = of_find_device_by_node(np); > + of_node_put(np); > + > + if (!pdev) > + return ERR_PTR(-ENODEV); > + } > + > + onboard_dev = dev_get_drvdata(>dev); > + put_device(>dev); > + > + /* > + * The presence of drvdata ('hub') indicates that the platform driver drop "('hub')" > diff --git a/drivers/usb/misc/onboard_usb_hub_pdevs.c > b/drivers/usb/misc/onboard_usb_dev_pdevs.c > similarity index 69% > rename from drivers/usb/misc/onboard_usb_hub_pdevs.c > rename to drivers/usb/misc/onboard_usb_dev_pdevs.c > index ed22a18f4ab7..ca56f67393f1 100644 > --- a/drivers/usb/misc/onboard_usb_hub_pdevs.c > +++ b/drivers/usb/misc/onboard_usb_dev_pdevs.c > > ... > > /** > - * onboard_hub_create_pdevs -- create platform devices for onboard USB hubs > - * @parent_hub : parent hub to scan for connected onboard hubs > - * @pdev_list: list of onboard hub platform devices owned by the > parent hub > + * onboard_dev_create_pdevs -- create platform devices for onboard USB > devices > + * @parent_hub : parent hub to scan for connected onboard devices > + * @pdev_list: list of onboard platform devices owned by the parent > hub > * > - * Creates a platform device for each
Re: [PATCH] drm/msm/a6xx: specify UBWC config for sc7180
On Tue, Feb 20, 2024 at 5:12 PM Dmitry Baryshkov wrote: > > Historically the Adreno driver has not been updating memory > configuration registers on a618 (SC7180 platform) implying that the > default configuration is fine. After the rework performed in the commit > 8814455a0e54 ("drm/msm: Refactor UBWC config setting") the function > a6xx_calc_ubwc_config() still contained this shortcut and did not > calculate UBWC configuration. However the function which now actually > updates hardware registers, a6xx_set_ubwc_config(), doesn't contain such > check. > > Rather than adding the check to a6xx_set_ubwc_config(), fill in the > UBWC config for a618 (based on readings from SC7180). > > Reported-by: Leonard Lausen > Link: https://gitlab.freedesktop.org/drm/msm/-/issues/49 > Fixes: 8814455a0e54 ("drm/msm: Refactor UBWC config setting") > Cc: Connor Abbott > Signed-off-by: Dmitry Baryshkov Thanks! Reviewed-by: Connor Abbott > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index c9c55e2ea584..dc80e5940f51 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1292,9 +1292,8 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu > *gpu) > gpu->ubwc_config.ubwc_mode = 1; > } > > - /* a618 is using the hw default values */ > if (adreno_is_a618(gpu)) > - return; > + gpu->ubwc_config.highest_bank_bit = 14; > > if (adreno_is_a619_holi(gpu)) > gpu->ubwc_config.highest_bank_bit = 13; > > --- > base-commit: 41c177cf354126a22443b5c80cec9fdd313e67e1 > change-id: 20240220-fd-sc7180-explicit-ubwc-40953fa55947 > > Best regards, > -- > Dmitry Baryshkov >
[PATCH RESEND v2] drm/syncobj: handle NULL fence in syncobj_eventfd_entry_func
During syncobj_eventfd_entry_func, dma_fence_chain_find_seqno may set the fence to NULL if the given seqno is signaled and a later seqno has already been submitted. In that case, the eventfd should be signaled immediately which currently does not happen. This is a similar issue to the one addressed by b19926d4f3a6 ("drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence") As a fix, if the return value of dma_fence_chain_find_seqno indicates success but it sets the fence to NULL, we will assign a stub fence to ensure the following code still signals the eventfd. v1 -> v2: assign a stub fence instead of signaling the eventfd Signed-off-by: Erik Kurzinger Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd") --- drivers/gpu/drm/drm_syncobj.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 6c82138b2c70..cb5b5838ccf3 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1441,10 +1441,20 @@ syncobj_eventfd_entry_func(struct drm_syncobj *syncobj, /* This happens inside the syncobj lock */ fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, 1)); + if (!fence) + return; + ret = dma_fence_chain_find_seqno(, entry->point); - if (ret != 0 || !fence) { + if (ret != 0) { + /* The given seqno has not been submitted yet. */ dma_fence_put(fence); return; + } else if (!fence) { + /* If dma_fence_chain_find_seqno returns 0 but sets the fence +* to NULL, it implies that the given seqno is signaled and a +* later seqno has already been submitted. Assign a stub fence +* so that the eventfd still gets signaled below. */ + fence = dma_fence_get_stub(); } list_del_init(>node); -- 2.43.2
Re: [PATCH v7 3/3] drm/buddy: Add defragmentation support
On 21/02/2024 12:18, Arunpravin Paneer Selvam wrote: Add a function to support defragmentation. v1: - Defragment the memory beginning from min_order till the required memory space is available. v2(Matthew): - add amdgpu user for defragmentation - add a warning if the two blocks are incompatible on defragmentation - call full defragmentation in the fini() function - place a condition to test if min_order is equal to 0 - replace the list with safe_reverse() variant as we might remove the block from the list. Signed-off-by: Arunpravin Paneer Selvam Suggested-by: Matthew Auld --- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 17 +++- drivers/gpu/drm/drm_buddy.c | 93 +--- include/drm/drm_buddy.h | 3 + 3 files changed, 97 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index e494f5bf136a..cff8a526c622 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -533,8 +533,21 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man, min_block_size, >blocks, vres->flags); - if (unlikely(r)) - goto error_free_blocks; + if (unlikely(r)) { + if (r == -ENOSPC) { + drm_buddy_defrag(mm, min_block_size); + r = drm_buddy_alloc_blocks(mm, fpfn, + lpfn, + size, + min_block_size, + >blocks, + vres->flags); + if (unlikely(r)) + goto error_free_blocks; + } else { + goto error_free_blocks; + } + } if (size > remaining_size) remaining_size = 0; diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index 18e004fa39d3..56bd1560fbcd 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -203,6 +203,8 @@ void drm_buddy_fini(struct drm_buddy *mm) drm_block_free(mm, mm->roots[i]); } + drm_buddy_defrag(mm, mm->chunk_size << mm->max_order); I think this needs to be called higher up, otherwise we blow up with the WARN, plus we just freed the root(s). There is also the case with non-power-of-two VRAM size, in which case you get multiple roots and max_order is just the largest root and not entire address space. I guess do this in the loop above and use the root order instead? Also this should be done as part of the first patch and then in this patch it is just a case of exporting it. Every commit should ideally be functional by itself. + WARN_ON(mm->avail != mm->size); kfree(mm->roots); @@ -276,25 +278,39 @@ drm_get_buddy(struct drm_buddy_block *block) } EXPORT_SYMBOL(drm_get_buddy); -static void __drm_buddy_free(struct drm_buddy *mm, -struct drm_buddy_block *block) +static unsigned int __drm_buddy_free(struct drm_buddy *mm, +struct drm_buddy_block *block, +bool defrag) { + unsigned int order, block_order; struct drm_buddy_block *parent; + block_order = drm_buddy_block_order(block); + while ((parent = block->parent)) { - struct drm_buddy_block *buddy; + struct drm_buddy_block *buddy = NULL; buddy = __get_buddy(block); if (!drm_buddy_block_is_free(buddy)) break; - if (drm_buddy_block_is_clear(block) != - drm_buddy_block_is_clear(buddy)) - break; + if (!defrag) { + /* +* Check the block and its buddy clear state and exit +* the loop if they both have the dissimilar state. +*/ + if (drm_buddy_block_is_clear(block) != + drm_buddy_block_is_clear(buddy)) + break; - if (drm_buddy_block_is_clear(block)) - mark_cleared(parent); + if (drm_buddy_block_is_clear(block)) + mark_cleared(parent); + } + + WARN_ON(defrag && + (drm_buddy_block_is_clear(block) == +drm_buddy_block_is_clear(buddy)));
Re: [PATCH v2 1/3] drm/syncobj: call drm_syncobj_fence_add_wait when WAIT_AVAILABLE flag is set
It looks these these patches have still not been merged after one month, is there anything more that needs to be done for this to happen? On 1/25/24 10:12, Daniel Vetter wrote: > On Fri, Jan 19, 2024 at 08:32:06AM -0800, Erik Kurzinger wrote: >> When waiting for a syncobj timeline point whose fence has not yet been >> submitted with the WAIT_FOR_SUBMIT flag, a callback is registered using >> drm_syncobj_fence_add_wait and the thread is put to sleep until the >> timeout expires. If the fence is submitted before then, >> drm_syncobj_add_point will wake up the sleeping thread immediately which >> will proceed to wait for the fence to be signaled. >> >> However, if the WAIT_AVAILABLE flag is used instead, >> drm_syncobj_fence_add_wait won't get called, meaning the waiting thread >> will always sleep for the full timeout duration, even if the fence gets >> submitted earlier. If it turns out that the fence *has* been submitted >> by the time it eventually wakes up, it will still indicate to userspace >> that the wait completed successfully (it won't return -ETIME), but it >> will have taken much longer than it should have. >> >> To fix this, we must call drm_syncobj_fence_add_wait if *either* the >> WAIT_FOR_SUBMIT flag or the WAIT_AVAILABLE flag is set. The only >> difference being that with WAIT_FOR_SUBMIT we will also wait for the >> fence to be signaled after it has been submitted while with >> WAIT_AVAILABLE we will return immediately. >> >> IGT test patch: >> https://lists.freedesktop.org/archives/igt-dev/2024-January/067537.html >> >> v1 -> v2: adjust lockdep_assert_none_held_once condition >> >> Fixes: 01d6c3578379 ("drm/syncobj: add support for timeline point wait v8") >> Signed-off-by: Erik Kurzinger > > Yeah I think this series catches now all the corner cases I spotted in v1. > On the series: > > Reviewed-by: Daniel Vetter >> --- >> drivers/gpu/drm/drm_syncobj.c | 6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c >> index 94ebc71e5be5..97be8b140599 100644 >> --- a/drivers/gpu/drm/drm_syncobj.c >> +++ b/drivers/gpu/drm/drm_syncobj.c >> @@ -1058,7 +1058,8 @@ static signed long >> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, >> uint64_t *points; >> uint32_t signaled_count, i; >> >> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) >> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | >> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) >> lockdep_assert_none_held_once(); >> >> points = kmalloc_array(count, sizeof(*points), GFP_KERNEL); >> @@ -1127,7 +1128,8 @@ static signed long >> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, >> * fallthough and try a 0 timeout wait! >> */ >> >> -if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { >> +if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | >> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) { >> for (i = 0; i < count; ++i) >> drm_syncobj_fence_add_wait(syncobjs[i], [i]); >> } >> -- >> 2.43.0 >> >
[linux-next:master] BUILD REGRESSION 4893c639cc3659cefaa675bf1e59f4e7571afb5c
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 4893c639cc3659cefaa675bf1e59f4e7571afb5c Add linux-next specific files for 20240221 Error/Warning: (recently discovered and may have been fixed) ld.lld: error: undefined symbol: ipt_alloc_initial_table ld.lld: error: undefined symbol: ipt_do_table ld.lld: error: undefined symbol: ipt_register_table ld.lld: error: undefined symbol: ipt_unregister_table_exit Unverified Error/Warning (likely false positive, please contact us if interested): fs/pidfs.c:98 pidfd_show_fdinfo() error: 'pid' dereferencing possible ERR_PTR() mm/userfaultfd.c:740 mfill_atomic() warn: inconsistent returns '>map_changing_lock'. Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- alpha-randconfig-r121-20240221 | `-- io_uring-io_uring.c:sparse:sparse:cast-to-restricted-io_req_flags_t |-- arc-allmodconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- arc-allyesconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- arm-allmodconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- arm-allyesconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- arm64-defconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- arm64-randconfig-002-20240221 | `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size |-- arm64-randconfig-003-20240221 | `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size |-- csky-allmodconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- csky-allyesconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- csky-randconfig-002-20240221 | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- i386-allmodconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- i386-allyesconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- i386-buildonly-randconfig-001-20240221 | `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size |-- i386-randconfig-001-20240221 | `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size |-- i386-randconfig-011-20240221 | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- i386-randconfig-013-20240221 | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- i386-randconfig-051-20240221 | `-- fs-ntfs3-frecord.c:warning:unused-variable-i_size |-- i386-randconfig-062-20240221 | |-- io_uring-io_uring.c:sparse:sparse:cast-to-restricted-io_req_flags_t | |-- sound-core-sound_kunit.c:sparse:sparse:incorrect-type-in-argument-(different-base-types)-expected-restricted-snd_pcm_format_t-usertype-format-got-int | `-- sound-core-sound_kunit.c:sparse:sparse:restricted-snd_pcm_format_t-degrades-to-integer |-- i386-randconfig-141-20240221 | |-- drivers-bluetooth-btintel.c-btintel_read_version()-warn:passing-zero-to-PTR_ERR | `-- mm-page_owner.c-stack_print()-warn:unsigned-nr_entries-is-never-less-than-zero. |-- loongarch-allmodconfig | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- loongarch-randconfig-r064-20240221 | `-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg |-- loongarch-randconfig-r111-20240221 | |-- drivers-gpu-drm-nouveau-nvkm-subdev-gsp-r535.c:warning:Function-parameter-or-struct-member-gsp-not-described-in-nvkm_gsp_radix3_sg | |-- drivers-leds-flash-leds-ktd2692.c:sparse:sparse:symbol-ktd2692_timing-was-not-declared.-Should-it-be-static | |-- include-trace-..-..-drivers-bus-mhi-host-trace.h:sparse:sparse:cast-to-restricted-__le32 | |-- include-trace-..-..-drivers-bus-mhi-host-trace.h:sparse:sparse:cast-to-restricted-__le64 | |-- include-trace-..-..-drivers-bus-mhi-host-trace.h:sparse:sparse:restricted-__l
Re: [PATCH v1] drivers/i915/intel_bios: Fix parsing backlight BDB data
On Tue, Feb 20, 2024 at 02:12:57PM -0700, Karthikeyan Ramasubramanian wrote: > Starting BDB version 239, hdr_dpcd_refresh_timeout is introduced to > backlight BDB data. Commit 700034566d68 ("drm/i915/bios: Define more BDB > contents") updated the backlight BDB data accordingly. This broke the > parsing of backlight BDB data in VBT for versions 236 - 238 (both > inclusive) and hence the backlight controls are not responding on units > with the concerned BDB version. > > backlight_control information has been present in backlight BDB data > from at least BDB version 191 onwards, if not before. Hence this patch > extracts the backlight_control information if the block size of > backlight BDB is >= version 191 backlight BDB block size. > Tested on Chromebooks using Jasperlake SoC (reports bdb->version = 236). > Tested on Chromebooks using Raptorlake SoC (reports bdb->version = 251). > > Fixes: 700034566d68 ("drm/i915/bios: Define more BDB contents") > Cc: sta...@vger.kernel.org > Cc: Jani Nikula > Cc: Ville Syrjälä > Signed-off-by: Karthikeyan Ramasubramanian > --- > > drivers/gpu/drm/i915/display/intel_bios.c | 22 +-- > drivers/gpu/drm/i915/display/intel_vbt_defs.h | 2 -- > 2 files changed, 6 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > b/drivers/gpu/drm/i915/display/intel_bios.c > index aa169b0055e97..4ec50903b9e64 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -1041,23 +1041,13 @@ parse_lfp_backlight(struct drm_i915_private *i915, > > panel->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI; > panel->vbt.backlight.controller = 0; > - if (i915->display.vbt.version >= 191) { > - size_t exp_size; > + if (i915->display.vbt.version >= 191 && > + get_blocksize(backlight_data) >= EXP_BDB_LFP_BL_DATA_SIZE_REV_191) { The size checks looks like nonsense to me. I guess maybe we needed it before we were guaranteed to have the full struct's worth of memory. But there should be no need for this anymore. > + const struct lfp_backlight_control_method *method; > > - if (i915->display.vbt.version >= 236) > - exp_size = sizeof(struct bdb_lfp_backlight_data); > - else if (i915->display.vbt.version >= 234) > - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_234; > - else > - exp_size = EXP_BDB_LFP_BL_DATA_SIZE_REV_191; > - > - if (get_blocksize(backlight_data) >= exp_size) { > - const struct lfp_backlight_control_method *method; > - > - method = _data->backlight_control[panel_type]; > - panel->vbt.backlight.type = method->type; > - panel->vbt.backlight.controller = method->controller; > - } > + method = _data->backlight_control[panel_type]; > + panel->vbt.backlight.type = method->type; > + panel->vbt.backlight.controller = method->controller; > } > > panel->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz; > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > index a9f44abfc9fc2..aeea5635a37ff 100644 > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > @@ -899,8 +899,6 @@ struct lfp_brightness_level { > > #define EXP_BDB_LFP_BL_DATA_SIZE_REV_191 \ > offsetof(struct bdb_lfp_backlight_data, brightness_level) > -#define EXP_BDB_LFP_BL_DATA_SIZE_REV_234 \ > - offsetof(struct bdb_lfp_backlight_data, brightness_precision_bits) > > struct bdb_lfp_backlight_data { > u8 entry_size; > -- > 2.44.0.rc0.258.g7320e95886-goog -- Ville Syrjälä Intel
Re: [PATCH] drm/amd/display: clean unnecessary braces
Hi Túlio, First of all thanks for your patch. See my comments inline. On 2/17/24 13:20, Túlio Fernandes wrote: Clean unnecessary braces in dc/dcn32/dcn32_resource_helpers.c and dc/dcn32/dcn201_link_encoder.c Did you identify this issue with checkpatch? If so, I recommend you paste the error message in the commit message. Iirc, checkpatch provides the file and the function name, which can make this commit message more informative. Also, wrap the commit message under 70 characters. Signed-off-by: Túlio Fernandes --- .../display/dc/dcn32/dcn32_resource_helpers.c| 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c index 87760600e154..e179dea148e7 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c @@ -110,14 +110,12 @@ uint32_t dcn32_helper_calculate_num_ways_for_subvp( struct dc_state *context) { if (context->bw_ctx.bw.dcn.mall_subvp_size_bytes > 0) { - if (dc->debug.force_subvp_num_ways) { + if (dc->debug.force_subvp_num_ways) return dc->debug.force_subvp_num_ways; - } else { + else return dcn32_helper_mall_bytes_to_ways(dc, context->bw_ctx.bw.dcn.mall_subvp_size_bytes); - } - } else { + } else Actually, we want to keep the braces around the else part to keep the braces balanced with the if condition. Thanks Siqueira return 0; - } } void dcn32_merge_pipes_for_subvp(struct dc *dc, @@ -250,9 +248,9 @@ bool dcn32_is_psr_capable(struct pipe_ctx *pipe) { bool psr_capable = false; - if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) { + if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) psr_capable = true; - } + return psr_capable; } @@ -278,9 +276,9 @@ static void override_det_for_subvp(struct dc *dc, struct dc_state *context, uint if (pipe_ctx->stream && pipe_ctx->plane_state && dc_state_get_pipe_subvp_type(context, pipe_ctx) != SUBVP_PHANTOM) { if (dcn32_allow_subvp_high_refresh_rate(dc, context, pipe_ctx)) { -if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) { + if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) fhd_count++; - } + subvp_high_refresh_count++; } }
Re: [PATCH v6 1/3] drm/buddy: Implement tracking clear page feature
On 21/02/2024 12:40, Paneer Selvam, Arunpravin wrote: On 2/16/2024 5:33 PM, Matthew Auld wrote: On 08/02/2024 15:49, Arunpravin Paneer Selvam wrote: - Add tracking clear page feature. - Driver should enable the DRM_BUDDY_CLEARED flag if it successfully clears the blocks in the free path. On the otherhand, DRM buddy marks each block as cleared. - Track the available cleared pages size - If driver requests cleared memory we prefer cleared memory but fallback to uncleared if we can't find the cleared blocks. when driver requests uncleared memory we try to use uncleared but fallback to cleared memory if necessary. - When a block gets freed we clear it and mark the freed block as cleared, when there are buddies which are cleared as well we can merge them. Otherwise, we prefer to keep the blocks as separated. v1: (Christian) - Depends on the flag check DRM_BUDDY_CLEARED, enable the block as cleared. Else, reset the clear flag for each block in the list. - For merging the 2 cleared blocks compare as below, drm_buddy_is_clear(block) != drm_buddy_is_clear(buddy) v2: (Matthew) - Add a wrapper drm_buddy_free_list_internal for the freeing of blocks operation within drm buddy. - Write a macro block_incompatible() to allocate the required blocks. - Update the xe driver for the drm_buddy_free_list change in arguments. Signed-off-by: Arunpravin Paneer Selvam Signed-off-by: Matthew Auld Suggested-by: Christian König Probably needs a new unit test. I think we are missing something to forcefully re-merge everything at fini()? In theory we can just call the defrag routine. Otherwise we might trigger various warnings since the root(s) might still be split. Also one nit below. Otherwise I think looks good. --- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +- drivers/gpu/drm/drm_buddy.c | 192 ++ drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 6 +- drivers/gpu/drm/tests/drm_buddy_test.c | 10 +- drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 4 +- include/drm/drm_buddy.h | 18 +- 6 files changed, 187 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index 8db880244324..c0c851409241 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -571,7 +571,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man, return 0; error_free_blocks: - drm_buddy_free_list(mm, >blocks); + drm_buddy_free_list(mm, >blocks, 0); mutex_unlock(>lock); error_fini: ttm_resource_fini(man, >base); @@ -604,7 +604,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man, amdgpu_vram_mgr_do_reserve(man); - drm_buddy_free_list(mm, >blocks); + drm_buddy_free_list(mm, >blocks, 0); mutex_unlock(>lock); atomic64_sub(vis_usage, >vis_usage); @@ -912,7 +912,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device *adev) kfree(rsv); list_for_each_entry_safe(rsv, temp, >reserved_pages, blocks) { - drm_buddy_free_list(>mm, >allocated); + drm_buddy_free_list(>mm, >allocated, 0); kfree(rsv); } if (!adev->gmc.is_app_apu) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index f57e6d74fb0e..33ad0cfbd54c 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -57,6 +57,16 @@ static void list_insert_sorted(struct drm_buddy *mm, __list_add(>link, node->link.prev, >link); } +static void clear_reset(struct drm_buddy_block *block) +{ + block->header &= ~DRM_BUDDY_HEADER_CLEAR; +} + +static void mark_cleared(struct drm_buddy_block *block) +{ + block->header |= DRM_BUDDY_HEADER_CLEAR; +} + static void mark_allocated(struct drm_buddy_block *block) { block->header &= ~DRM_BUDDY_HEADER_STATE; @@ -223,6 +233,12 @@ static int split_block(struct drm_buddy *mm, mark_free(mm, block->left); mark_free(mm, block->right); + if (drm_buddy_block_is_clear(block)) { + mark_cleared(block->left); + mark_cleared(block->right); + clear_reset(block); + } + mark_split(block); return 0; @@ -273,6 +289,13 @@ static void __drm_buddy_free(struct drm_buddy *mm, if (!drm_buddy_block_is_free(buddy)) break; + if (drm_buddy_block_is_clear(block) != + drm_buddy_block_is_clear(buddy)) + break; + + if (drm_buddy_block_is_clear(block)) + mark_cleared(parent); + list_del(>link); drm_block_free(mm, block); @@ -295,26 +318,61 @@ void drm_buddy_free_block(struct drm_buddy *mm, { BUG_ON(!drm_buddy_block_is_allocated(block)); mm->avail += drm_buddy_block_size(mm, block); + if (drm_buddy_block_is_clear(block)) + mm->clear_avail +=
[PATCH] drm/mediatek: Add MT8188 Overlay Driver Data
Add MT8188 overlay driver configuration data. This change consequently enables 10-bit overlay support on MT8188 devices. Tested by running ChromeOS UI on MT8188 and using modetest -P. AR30 and BA30 overlays are confirmed to work from modetest. Signed-off-by: Justin Green Tested-by: Justin Green --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 2bffe4245466..696aabe124c2 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -635,6 +635,17 @@ static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = { .supports_clrfmt_ext = true, }; +static const struct mtk_disp_ovl_data mt8188_ovl_driver_data = { + .addr = DISP_REG_OVL_ADDR_MT8173, + .gmc_bits = 10, + .layer_nr = 4, + .fmt_rgb565_is_0 = true, + .smi_id_en = true, + .formats = mt8195_formats, + .num_formats = ARRAY_SIZE(mt8195_formats), + .supports_clrfmt_ext = true, +}; + static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = { { .compatible = "mediatek,mt2701-disp-ovl", .data = _ovl_driver_data}, @@ -650,6 +661,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = { .data = _ovl_2l_driver_data}, { .compatible = "mediatek,mt8195-disp-ovl", .data = _ovl_driver_data}, + { .compatible = "mediatek,mt8188-disp-ovl", + .data = _ovl_driver_data}, {}, }; MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match); -- 2.44.0.rc0.258.g7320e95886-goog
Re: [PATCH] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs
On 21/02/2024 16:12, Adrián Larumbe wrote: > Debugfs isn't always available in production builds that try to squeeze > every single byte out of the kernel image, but we still need a way to > toggle the timestamp and cycle counter registers so that jobs can be > profiled for fdinfo's drm engine and cycle calculations. > > Drop the debugfs knob and replace it with a sysfs file that accomplishes > the same functionality, and document its ABI in a separate file. > > Signed-off-by: Adrián Larumbe > --- > .../testing/sysfs-driver-panfrost-profiling | 10 +++ > Documentation/gpu/panfrost.rst| 9 +++ > drivers/gpu/drm/panfrost/Makefile | 5 +- > drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21 -- > drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 > drivers/gpu/drm/panfrost/panfrost_device.h| 5 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 14 ++-- > drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- > drivers/gpu/drm/panfrost/panfrost_sysfs.c | 74 +++ > drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 > 10 files changed, 124 insertions(+), 45 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-panfrost-profiling > delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c > delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h > create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.c > create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.h > > diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling > b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling > new file mode 100644 > index ..ce54069714f3 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling > @@ -0,0 +1,10 @@ > +What: > /sys/bus/.../drivers/panfrost/.../drm/../profiling/status > +Date:February 2024 > +KernelVersion: 6.8.0 > +Contact: Adrian Larumbe > +Description: > +Get/set drm fdinfo's engine and cycles profiling status. > +Valid values are: > + 0: Disable fdinfo job profiling sources. This disables both the > GPU's > +timestamp and cycle counter registers. > + 1: Enable the above. Minor point, but if we're going to eventually come up with a generic way of doing this, then we're going to have to think about backwards compatibility for this sysfs file. I would expect in this new world '0' would mean "default behaviour; off unless the new-fangled thing enables profiling" and '1' means "force on". In which case perhaps wording like the below would be clearer: 0: Don't enable fdinfo job profiling sources. 1: Enable fdinfo job profiling sources, this enables both the GPU's timestamp and cycle counter registers. Or am I being too picky over the wording ;) One other small issue below... > diff --git a/Documentation/gpu/panfrost.rst b/Documentation/gpu/panfrost.rst > index b80e41f4b2c5..be4ac282ef63 100644 > --- a/Documentation/gpu/panfrost.rst > +++ b/Documentation/gpu/panfrost.rst > @@ -38,3 +38,12 @@ the currently possible format options: > > Possible `drm-engine-` key names are: `fragment`, and `vertex-tiler`. > `drm-curfreq-` values convey the current operating frequency for that engine. > + > +Users must bear in mind that engine and cycle sampling are disabled by > default, > +because of power saving concerns. `fdinfo` users and benchmark applications > which > +query the fdinfo file must make sure to toggle the job profiling status of > the > +driver by writing into the appropriate sysfs node:: > + > +echo > > /sys/bus/platform/drivers/panfrost/[a-f0-9]*.gpu/drm/card1/profiling > + > +Where `N` is either `0` or `1`, depending on the desired enablement status. > diff --git a/drivers/gpu/drm/panfrost/Makefile > b/drivers/gpu/drm/panfrost/Makefile > index 2c01c1e7523e..6e718595d8a6 100644 > --- a/drivers/gpu/drm/panfrost/Makefile > +++ b/drivers/gpu/drm/panfrost/Makefile > @@ -10,8 +10,7 @@ panfrost-y := \ > panfrost_job.o \ > panfrost_mmu.o \ > panfrost_perfcnt.o \ > - panfrost_dump.o > - > -panfrost-$(CONFIG_DEBUG_FS) += panfrost_debugfs.o > + panfrost_dump.o \ > + panfrost_sysfs.o > > obj-$(CONFIG_DRM_PANFROST) += panfrost.o > diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.c > b/drivers/gpu/drm/panfrost/panfrost_debugfs.c > deleted file mode 100644 > index 72d4286a6bf7.. > --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.c > +++ /dev/null > @@ -1,21 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -/* Copyright 2023 Collabora ltd. */ > -/* Copyright 2023 Amazon.com, Inc. or its affiliates. */ > - > -#include > -#include > -#include > -#include > -#include > - > -#include "panfrost_device.h" > -#include "panfrost_gpu.h" > -#include "panfrost_debugfs.h" > - > -void panfrost_debugfs_init(struct drm_minor *minor) > -{ > - struct drm_device *dev =
Re: [PATCH v2 01/10] backlight: Match backlight device against struct fb_info.bl_dev
On Wed, Feb 21, 2024 at 5:45 PM Thomas Zimmermann wrote: > Am 21.02.24 um 15:34 schrieb Andy Shevchenko: > > On Wed, Feb 21, 2024 at 10:41:28AM +0100, Thomas Zimmermann wrote: > >> Framebuffer drivers for devices with dedicated backlight are supposed > >> to set struct fb_info.bl_dev to the backlight's respective device. Use > >> the value to match backlight and framebuffer in the backlight core code. ... > >> if (!bd->ops) > >> goto out; > >> -if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) > >> +else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) > > What's the point of adding redundant 'else'? > > > >> goto out; > >> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) > >> +else if (info->bl_dev && info->bl_dev != bd) > > Ditto. > > They group these tests into one single block of code; signaling that > these tests serve the same purpose. Commit message has nothing about this. Also if needed, it should be a separate change. > >> +goto out; > >> +#endif -- With Best Regards, Andy Shevchenko
Re: [PATCH 8/9] media: dt-bindings: Add Intel Displayport RX IP
On Thu, Feb 15, 2024 at 6:26 PM Conor Dooley wrote: > > Yo, > > On Mon, Feb 12, 2024 at 01:13:22PM +, Paweł Anikiel wrote: > > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP > > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video > > capture and Multi-Stream Transport. The user guide can be found here: > > > > https://www.intel.com/programmable/technical-pdfs/683273.pdf > > > > Signed-off-by: Paweł Anikiel > > --- > > .../devicetree/bindings/media/intel,dprx.yaml | 125 ++ > > 1 file changed, 125 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml > > b/Documentation/devicetree/bindings/media/intel,dprx.yaml > > new file mode 100644 > > index ..3ed37e0a4a94 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml > > @@ -0,0 +1,125 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/intel,dprx.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Intel DisplayPort RX IP > > + > > +maintainers: > > + - Paweł Anikiel > > + > > +properties: > > + compatible: > > +const: intel,dprx > > Please version this compatible, given that is it for an FPGA IP. > I could not find an example of another intel IP that had versioning, but > there's plenty of xilinx stuff you can get inspiration from. The IP has had a few different revisions, so I decided to just use the "IP version" which is 20.0.1 for the version this driver is targeting. > > > + reg: > > +items: > > + - description: core registers > > + - description: irq registers > > + > > + interrupts: > > +maxItems: 1 > > + > > + intel,has-mst: > > Mostly this looks fine, but this property drew my eye. > Firstly, I'd probably call this "intel,multi-stream-support" rather than > "intel,has-mst". Sure, > > > +type: boolean > > +description: The device supports Multi-Stream Transport > > Secondly, there are many many configuration parameters for this IP, > but you have chosen to document just one. > Are all other configuration parameters currently in their default > states or ignored by the driver? If not, please at least document all > configuration settings that you rely on - for example the max stream > count or audio packet encoding. I looked at all the parameters listed in the user guide, and most of them don't affect the driver. I listed the ones which are required, and added support for the remaining ones in v2. > > > + > > + port: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: SST main link > > + > > + ports: > > +$ref: /schemas/graph.yaml#/properties/ports > > + > > +properties: > > + port@0: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: MST virtual channel 0 or SST main link > > + > > + port@1: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: MST virtual channel 1 > > + > > + port@2: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: MST virtual channel 2 > > + > > + port@3: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: MST virtual channel 3 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +allOf: > > + - if: > > + required: > > +- intel,has-mst > > +then: > > + required: > > +- ports > > +else: > > + required: > > +- port > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > +dprx@c0062000 { > > "dprx" isn't a class of device, please try to use a generic node name > here. I couldn't find anything in the DT spec or any other similar examples, so I chose dp-receiver.
Re: [PATCH v17 0/5] Add RZ/{G2L, G2LC} and RZ/V2L Display Unit support
On Sun, 18 Feb 2024 16:48:35 +, Biju Das wrote: > This path series aims to add support for RZ/G2L DU DRM driver. > > RZ/G2L LCD controller composed of Frame compression Processor(FCPVD), Video > signal processor (VSPD) and Display unit(DU). The output of LCDC is > connected to Display parallel interface and MIPI link video interface. > > The output from DSI is connected to ADV7535. > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH v15 0/5] Add RZ/{G2L,G2LC} and RZ/V2L Display Unit support
On Tue, 28 Nov 2023 10:51:24 +, Biju Das wrote: > This path series aims to add support for RZ/G2L DU DRM driver. > > RZ/G2L LCD controller composed of Frame compression Processor(FCPVD), Video > signal processor (VSPD) and Display unit(DU). The output of LCDC is > connected to Display parallel interface and MIPI link video interface. > > The output from DSI is connected to ADV7535. > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH 7/9] media: dt-bindings: Add Chameleon v3 framebuffer
On Thu, Feb 15, 2024 at 6:29 PM Conor Dooley wrote: > > On Mon, Feb 12, 2024 at 01:13:21PM +, Paweł Anikiel wrote: > > The Chameleon v3 uses the framebuffer IP core to take the video signal > > from different sources and directly write frames into memory. > > > > Signed-off-by: Paweł Anikiel > > --- > > .../bindings/media/google,chv3-fb.yaml| 77 +++ > > 1 file changed, 77 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/media/google,chv3-fb.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/google,chv3-fb.yaml > > b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml > > new file mode 100644 > > index ..ba6643cc7232 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml > > @@ -0,0 +1,77 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/google,chv3-fb.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Google Chameleon v3 video framebuffer > > + > > +maintainers: > > + - Paweł Anikiel > > + > > +properties: > > + compatible: > > +const: google,chv3-fb > > + > > + reg: > > +items: > > + - description: core registers > > + - description: irq registers > > + > > + interrupts: > > +maxItems: 1 > > + > > + google,legacy-format: > > +type: boolean > > +description: The incoming video stream is in 32-bit padded mode. > > + > > + google,no-endpoint: > > +type: boolean > > +description: > > + The framebuffer isn't connected to a controllable endpoint. > > + The video interface still works, but EDID control is unavailable > > + and DV timing information only reports the active video width/height. > > Why does this need a dedicated property? Is it not sufficient to check > that there are no endpoints in the devicetree? Yes, I think it is sufficient. I removed the property and added a check in the driver in v2.
[PATCH 3/3] arch: Rename fbdev header and source files
The per-architecture fbdev code has no dependencies on fbdev and can be used for any video-related subsystem. Rename the files to 'video'. Use video-sti.c on parisc as the source file depends on CONFIG_STI_CORE. Further update all includes statements, includ guards, and Makefiles. Also update a few strings and comments to refer to video instead of fbdev. Signed-off-by: Thomas Zimmermann Cc: Vineet Gupta Cc: Catalin Marinas Cc: Will Deacon Cc: Huacai Chen Cc: WANG Xuerui Cc: Geert Uytterhoeven Cc: Thomas Bogendoerfer Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Yoshinori Sato Cc: Rich Felker Cc: John Paul Adrian Glaubitz Cc: "David S. Miller" Cc: Andreas Larsson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: x...@kernel.org Cc: "H. Peter Anvin" --- arch/arc/include/asm/fb.h| 8 arch/arc/include/asm/video.h | 8 arch/arm/include/asm/fb.h| 6 -- arch/arm/include/asm/video.h | 6 ++ arch/arm64/include/asm/fb.h | 10 -- arch/arm64/include/asm/video.h | 10 ++ arch/loongarch/include/asm/{fb.h => video.h} | 8 arch/m68k/include/asm/{fb.h => video.h} | 8 arch/mips/include/asm/{fb.h => video.h} | 12 ++-- arch/parisc/include/asm/{fb.h => video.h}| 8 arch/parisc/video/Makefile | 2 +- arch/parisc/video/{fbdev.c => video-sti.c} | 2 +- arch/powerpc/include/asm/{fb.h => video.h} | 8 arch/powerpc/kernel/pci-common.c | 2 +- arch/sh/include/asm/fb.h | 7 --- arch/sh/include/asm/video.h | 7 +++ arch/sparc/include/asm/{fb.h => video.h} | 8 arch/sparc/video/Makefile| 2 +- arch/sparc/video/{fbdev.c => video.c}| 4 ++-- arch/x86/include/asm/{fb.h => video.h} | 8 arch/x86/video/Makefile | 2 +- arch/x86/video/{fbdev.c => video.c} | 3 ++- include/asm-generic/Kbuild | 2 +- include/asm-generic/{fb.h => video.h}| 6 +++--- include/linux/fb.h | 2 +- 25 files changed, 75 insertions(+), 74 deletions(-) delete mode 100644 arch/arc/include/asm/fb.h create mode 100644 arch/arc/include/asm/video.h delete mode 100644 arch/arm/include/asm/fb.h create mode 100644 arch/arm/include/asm/video.h delete mode 100644 arch/arm64/include/asm/fb.h create mode 100644 arch/arm64/include/asm/video.h rename arch/loongarch/include/asm/{fb.h => video.h} (86%) rename arch/m68k/include/asm/{fb.h => video.h} (86%) rename arch/mips/include/asm/{fb.h => video.h} (76%) rename arch/parisc/include/asm/{fb.h => video.h} (68%) rename arch/parisc/video/{fbdev.c => video-sti.c} (96%) rename arch/powerpc/include/asm/{fb.h => video.h} (76%) delete mode 100644 arch/sh/include/asm/fb.h create mode 100644 arch/sh/include/asm/video.h rename arch/sparc/include/asm/{fb.h => video.h} (89%) rename arch/sparc/video/{fbdev.c => video.c} (86%) rename arch/x86/include/asm/{fb.h => video.h} (77%) rename arch/x86/video/{fbdev.c => video.c} (97%) rename include/asm-generic/{fb.h => video.h} (96%) diff --git a/arch/arc/include/asm/fb.h b/arch/arc/include/asm/fb.h deleted file mode 100644 index 9c2383d29cbb9..0 --- a/arch/arc/include/asm/fb.h +++ /dev/null @@ -1,8 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ - -#ifndef _ASM_FB_H_ -#define _ASM_FB_H_ - -#include - -#endif /* _ASM_FB_H_ */ diff --git a/arch/arc/include/asm/video.h b/arch/arc/include/asm/video.h new file mode 100644 index 0..8ff7263727fe7 --- /dev/null +++ b/arch/arc/include/asm/video.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef _ASM_VIDEO_H_ +#define _ASM_VIDEO_H_ + +#include + +#endif /* _ASM_VIDEO_H_ */ diff --git a/arch/arm/include/asm/fb.h b/arch/arm/include/asm/fb.h deleted file mode 100644 index ce20a43c30339..0 --- a/arch/arm/include/asm/fb.h +++ /dev/null @@ -1,6 +0,0 @@ -#ifndef _ASM_FB_H_ -#define _ASM_FB_H_ - -#include - -#endif /* _ASM_FB_H_ */ diff --git a/arch/arm/include/asm/video.h b/arch/arm/include/asm/video.h new file mode 100644 index 0..f570565366e67 --- /dev/null +++ b/arch/arm/include/asm/video.h @@ -0,0 +1,6 @@ +#ifndef _ASM_VIDEO_H_ +#define _ASM_VIDEO_H_ + +#include + +#endif /* _ASM_VIDEO_H_ */ diff --git a/arch/arm64/include/asm/fb.h b/arch/arm64/include/asm/fb.h deleted file mode 100644 index 1a495d8fb2ce0..0 --- a/arch/arm64/include/asm/fb.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Copyright (C) 2012 ARM Ltd. - */ -#ifndef __ASM_FB_H_ -#define __ASM_FB_H_ - -#include - -#endif /* __ASM_FB_H_ */ diff --git a/arch/arm64/include/asm/video.h b/arch/arm64/include/asm/video.h new file mode 100644 index
[PATCH 1/3] arch: Select fbdev helpers with CONFIG_VIDEO
Various Kconfig options selected the per-architecture helpers for fbdev. But none of the contained code depends on fbdev. Standardize on CONFIG_VIDEO, which will allow to add more general helpers for video functionality. CONFIG_VIDEO protects each architecture's video/ directory. This allows for the use of more fine-grained control for each directory's files, such as the use of CONFIG_STI_CORE on parisc. Signed-off-by: Thomas Zimmermann Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: "David S. Miller" Cc: Andreas Larsson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: x...@kernel.org Cc: "H. Peter Anvin" --- arch/parisc/Makefile | 2 +- arch/sparc/Makefile | 4 ++-- arch/sparc/video/Makefile | 2 +- arch/x86/Makefile | 2 +- arch/x86/video/Makefile | 3 ++- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile index 316f84f1d15c8..21b8166a68839 100644 --- a/arch/parisc/Makefile +++ b/arch/parisc/Makefile @@ -119,7 +119,7 @@ export LIBGCC libs-y += arch/parisc/lib/ $(LIBGCC) -drivers-y += arch/parisc/video/ +drivers-$(CONFIG_VIDEO) += arch/parisc/video/ boot := arch/parisc/boot diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile index 5f60359361312..757451c3ea1df 100644 --- a/arch/sparc/Makefile +++ b/arch/sparc/Makefile @@ -59,8 +59,8 @@ endif libs-y += arch/sparc/prom/ libs-y += arch/sparc/lib/ -drivers-$(CONFIG_PM) += arch/sparc/power/ -drivers-$(CONFIG_FB) += arch/sparc/video/ +drivers-$(CONFIG_PM)+= arch/sparc/power/ +drivers-$(CONFIG_VIDEO) += arch/sparc/video/ boot := arch/sparc/boot diff --git a/arch/sparc/video/Makefile b/arch/sparc/video/Makefile index 6baddbd58e4db..9dd82880a027a 100644 --- a/arch/sparc/video/Makefile +++ b/arch/sparc/video/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_FB) += fbdev.o +obj-y += fbdev.o diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 15a5f4f2ff0aa..c0ea612c62ebe 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -265,7 +265,7 @@ drivers-$(CONFIG_PCI)+= arch/x86/pci/ # suspend and hibernation support drivers-$(CONFIG_PM) += arch/x86/power/ -drivers-$(CONFIG_FB_CORE) += arch/x86/video/ +drivers-$(CONFIG_VIDEO) += arch/x86/video/ # boot loader support. Several targets are kept for legacy purposes diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile index 5ebe48752ffc4..9dd82880a027a 100644 --- a/arch/x86/video/Makefile +++ b/arch/x86/video/Makefile @@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_FB_CORE) += fbdev.o + +obj-y += fbdev.o -- 2.43.0
[PATCH 2/3] arch: Remove struct fb_info from video helpers
The per-architecture video helpers do not depend on struct fb_info or anything else from fbdev. Remove it from the interface and replace fb_is_primary_device() with video_is_primary_device(). The new helper is similar in functionality, but can operate on non-fbdev devices. Signed-off-by: Thomas Zimmermann Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: "David S. Miller" Cc: Andreas Larsson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: x...@kernel.org Cc: "H. Peter Anvin" --- arch/parisc/include/asm/fb.h | 8 +--- arch/parisc/video/fbdev.c| 9 + arch/sparc/include/asm/fb.h | 7 --- arch/sparc/video/fbdev.c | 17 - arch/x86/include/asm/fb.h| 8 +--- arch/x86/video/fbdev.c | 18 +++--- drivers/video/fbdev/core/fbcon.c | 2 +- include/asm-generic/fb.h | 11 ++- 8 files changed, 41 insertions(+), 39 deletions(-) diff --git a/arch/parisc/include/asm/fb.h b/arch/parisc/include/asm/fb.h index 658a8a7dc5312..ed2a195a3e762 100644 --- a/arch/parisc/include/asm/fb.h +++ b/arch/parisc/include/asm/fb.h @@ -2,11 +2,13 @@ #ifndef _ASM_FB_H_ #define _ASM_FB_H_ -struct fb_info; +#include + +struct device; #if defined(CONFIG_STI_CORE) -int fb_is_primary_device(struct fb_info *info); -#define fb_is_primary_device fb_is_primary_device +bool video_is_primary_device(struct device *dev); +#define video_is_primary_device video_is_primary_device #endif #include diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c index e4f8ac99fc9e0..540fa0c919d59 100644 --- a/arch/parisc/video/fbdev.c +++ b/arch/parisc/video/fbdev.c @@ -5,12 +5,13 @@ * Copyright (C) 2001-2002 Thomas Bogendoerfer */ -#include #include #include -int fb_is_primary_device(struct fb_info *info) +#include + +bool video_is_primary_device(struct device *dev) { struct sti_struct *sti; @@ -21,6 +22,6 @@ int fb_is_primary_device(struct fb_info *info) return true; /* return true if it's the default built-in framebuffer driver */ - return (sti->dev == info->device); + return (sti->dev == dev); } -EXPORT_SYMBOL(fb_is_primary_device); +EXPORT_SYMBOL(video_is_primary_device); diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h index 24440c0fda490..07f0325d6921c 100644 --- a/arch/sparc/include/asm/fb.h +++ b/arch/sparc/include/asm/fb.h @@ -3,10 +3,11 @@ #define _SPARC_FB_H_ #include +#include #include -struct fb_info; +struct device; #ifdef CONFIG_SPARC32 static inline pgprot_t pgprot_framebuffer(pgprot_t prot, @@ -18,8 +19,8 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot, #define pgprot_framebuffer pgprot_framebuffer #endif -int fb_is_primary_device(struct fb_info *info); -#define fb_is_primary_device fb_is_primary_device +bool video_is_primary_device(struct device *dev); +#define video_is_primary_device video_is_primary_device static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n) { diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c index bff66dd1909a4..e46f0499c2774 100644 --- a/arch/sparc/video/fbdev.c +++ b/arch/sparc/video/fbdev.c @@ -1,26 +1,25 @@ // SPDX-License-Identifier: GPL-2.0 #include -#include +#include #include +#include #include -int fb_is_primary_device(struct fb_info *info) +bool video_is_primary_device(struct device *dev) { - struct device *dev = info->device; - struct device_node *node; + struct device_node *node = dev->of_node; if (console_set_on_cmdline) - return 0; + return false; - node = dev->of_node; if (node && node == of_console_device) - return 1; + return true; - return 0; + return false; } -EXPORT_SYMBOL(fb_is_primary_device); +EXPORT_SYMBOL(video_is_primary_device); MODULE_DESCRIPTION("Sparc fbdev helpers"); MODULE_LICENSE("GPL"); diff --git a/arch/x86/include/asm/fb.h b/arch/x86/include/asm/fb.h index c3b9582de7efd..999db33792869 100644 --- a/arch/x86/include/asm/fb.h +++ b/arch/x86/include/asm/fb.h @@ -2,17 +2,19 @@ #ifndef _ASM_X86_FB_H #define _ASM_X86_FB_H +#include + #include -struct fb_info; +struct device; pgprot_t pgprot_framebuffer(pgprot_t prot, unsigned long vm_start, unsigned long vm_end, unsigned long offset); #define pgprot_framebuffer pgprot_framebuffer -int fb_is_primary_device(struct fb_info *info); -#define fb_is_primary_device fb_is_primary_device +bool video_is_primary_device(struct device *dev); +#define video_is_primary_device video_is_primary_device #include diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c index 1dd6528cc947c..4d87ce8e257fe 100644 --- a/arch/x86/video/fbdev.c +++ b/arch/x86/video/fbdev.c @@ -7,7 +7,6 @@ * */ -#include #include
[PATCH 0/3] arch: Remove fbdev dependency from video helpers
Make architecture helpers for display functionality depend on general video functionality instead of fbdev. This avoid the dependency on fbdev and makes the functionality available for non-fbdev code. Patch 1 replaces the variety of Kconfig options that control the Makefiles with CONFIG_VIDEO. More fine-grained control of the build can then be done within each video/ directory; see sparc for an example. Patch 2 replaces fb_is_primary_device() with video_is_primary_device(), which has no dependencies on fbdev. The implementation remains identical on all affected platforms. There's one minor change in fbcon, which is the only caller of fb_is_primary_device(). Patch 3 renames the source and files from fbdev to video. Thomas Zimmermann (3): arch: Select fbdev helpers with CONFIG_VIDEO arch: Remove struct fb_info from video helpers arch: Rename fbdev header and source files arch/arc/include/asm/fb.h| 8 -- arch/arc/include/asm/video.h | 8 ++ arch/arm/include/asm/fb.h| 6 - arch/arm/include/asm/video.h | 6 + arch/arm64/include/asm/fb.h | 10 arch/arm64/include/asm/video.h | 10 arch/loongarch/include/asm/{fb.h => video.h} | 8 +++--- arch/m68k/include/asm/{fb.h => video.h} | 8 +++--- arch/mips/include/asm/{fb.h => video.h} | 12 - arch/parisc/Makefile | 2 +- arch/parisc/include/asm/fb.h | 14 --- arch/parisc/include/asm/video.h | 16 arch/parisc/video/Makefile | 2 +- arch/parisc/video/{fbdev.c => video-sti.c} | 9 --- arch/powerpc/include/asm/{fb.h => video.h} | 8 +++--- arch/powerpc/kernel/pci-common.c | 2 +- arch/sh/include/asm/fb.h | 7 -- arch/sh/include/asm/video.h | 7 ++ arch/sparc/Makefile | 4 +-- arch/sparc/include/asm/{fb.h => video.h} | 15 +-- arch/sparc/video/Makefile| 2 +- arch/sparc/video/fbdev.c | 26 arch/sparc/video/video.c | 25 +++ arch/x86/Makefile| 2 +- arch/x86/include/asm/fb.h| 19 -- arch/x86/include/asm/video.h | 21 arch/x86/video/Makefile | 3 ++- arch/x86/video/{fbdev.c => video.c} | 21 +++- drivers/video/fbdev/core/fbcon.c | 2 +- include/asm-generic/Kbuild | 2 +- include/asm-generic/{fb.h => video.h}| 17 +++-- include/linux/fb.h | 2 +- 32 files changed, 154 insertions(+), 150 deletions(-) delete mode 100644 arch/arc/include/asm/fb.h create mode 100644 arch/arc/include/asm/video.h delete mode 100644 arch/arm/include/asm/fb.h create mode 100644 arch/arm/include/asm/video.h delete mode 100644 arch/arm64/include/asm/fb.h create mode 100644 arch/arm64/include/asm/video.h rename arch/loongarch/include/asm/{fb.h => video.h} (86%) rename arch/m68k/include/asm/{fb.h => video.h} (86%) rename arch/mips/include/asm/{fb.h => video.h} (76%) delete mode 100644 arch/parisc/include/asm/fb.h create mode 100644 arch/parisc/include/asm/video.h rename arch/parisc/video/{fbdev.c => video-sti.c} (78%) rename arch/powerpc/include/asm/{fb.h => video.h} (76%) delete mode 100644 arch/sh/include/asm/fb.h create mode 100644 arch/sh/include/asm/video.h rename arch/sparc/include/asm/{fb.h => video.h} (75%) delete mode 100644 arch/sparc/video/fbdev.c create mode 100644 arch/sparc/video/video.c delete mode 100644 arch/x86/include/asm/fb.h create mode 100644 arch/x86/include/asm/video.h rename arch/x86/video/{fbdev.c => video.c} (66%) rename include/asm-generic/{fb.h => video.h} (89%) -- 2.43.0
[PATCH] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs
Debugfs isn't always available in production builds that try to squeeze every single byte out of the kernel image, but we still need a way to toggle the timestamp and cycle counter registers so that jobs can be profiled for fdinfo's drm engine and cycle calculations. Drop the debugfs knob and replace it with a sysfs file that accomplishes the same functionality, and document its ABI in a separate file. Signed-off-by: Adrián Larumbe --- .../testing/sysfs-driver-panfrost-profiling | 10 +++ Documentation/gpu/panfrost.rst| 9 +++ drivers/gpu/drm/panfrost/Makefile | 5 +- drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21 -- drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 drivers/gpu/drm/panfrost/panfrost_device.h| 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 14 ++-- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/panfrost/panfrost_sysfs.c | 74 +++ drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 10 files changed, 124 insertions(+), 45 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-panfrost-profiling delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.c create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.h diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling new file mode 100644 index ..ce54069714f3 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling @@ -0,0 +1,10 @@ +What: /sys/bus/.../drivers/panfrost/.../drm/../profiling/status +Date: February 2024 +KernelVersion: 6.8.0 +Contact: Adrian Larumbe +Description: +Get/set drm fdinfo's engine and cycles profiling status. +Valid values are: + 0: Disable fdinfo job profiling sources. This disables both the GPU's +timestamp and cycle counter registers. + 1: Enable the above. diff --git a/Documentation/gpu/panfrost.rst b/Documentation/gpu/panfrost.rst index b80e41f4b2c5..be4ac282ef63 100644 --- a/Documentation/gpu/panfrost.rst +++ b/Documentation/gpu/panfrost.rst @@ -38,3 +38,12 @@ the currently possible format options: Possible `drm-engine-` key names are: `fragment`, and `vertex-tiler`. `drm-curfreq-` values convey the current operating frequency for that engine. + +Users must bear in mind that engine and cycle sampling are disabled by default, +because of power saving concerns. `fdinfo` users and benchmark applications which +query the fdinfo file must make sure to toggle the job profiling status of the +driver by writing into the appropriate sysfs node:: + +echo > /sys/bus/platform/drivers/panfrost/[a-f0-9]*.gpu/drm/card1/profiling + +Where `N` is either `0` or `1`, depending on the desired enablement status. diff --git a/drivers/gpu/drm/panfrost/Makefile b/drivers/gpu/drm/panfrost/Makefile index 2c01c1e7523e..6e718595d8a6 100644 --- a/drivers/gpu/drm/panfrost/Makefile +++ b/drivers/gpu/drm/panfrost/Makefile @@ -10,8 +10,7 @@ panfrost-y := \ panfrost_job.o \ panfrost_mmu.o \ panfrost_perfcnt.o \ - panfrost_dump.o - -panfrost-$(CONFIG_DEBUG_FS) += panfrost_debugfs.o + panfrost_dump.o \ + panfrost_sysfs.o obj-$(CONFIG_DRM_PANFROST) += panfrost.o diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.c b/drivers/gpu/drm/panfrost/panfrost_debugfs.c deleted file mode 100644 index 72d4286a6bf7.. --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.c +++ /dev/null @@ -1,21 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* Copyright 2023 Collabora ltd. */ -/* Copyright 2023 Amazon.com, Inc. or its affiliates. */ - -#include -#include -#include -#include -#include - -#include "panfrost_device.h" -#include "panfrost_gpu.h" -#include "panfrost_debugfs.h" - -void panfrost_debugfs_init(struct drm_minor *minor) -{ - struct drm_device *dev = minor->dev; - struct panfrost_device *pfdev = platform_get_drvdata(to_platform_device(dev->dev)); - - debugfs_create_atomic_t("profile", 0600, minor->debugfs_root, >profile_mode); -} diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.h b/drivers/gpu/drm/panfrost/panfrost_debugfs.h deleted file mode 100644 index c5af5f35877f.. --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* - * Copyright 2023 Collabora ltd. - * Copyright 2023 Amazon.com, Inc. or its affiliates. - */ - -#ifndef PANFROST_DEBUGFS_H -#define PANFROST_DEBUGFS_H - -#ifdef CONFIG_DEBUG_FS -void panfrost_debugfs_init(struct drm_minor *minor); -#endif - -#endif /* PANFROST_DEBUGFS_H */ diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index
[PATCH v2 8/9] media: dt-bindings: Add Intel Displayport RX IP
The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video capture and Multi-Stream Transport. The user guide can be found here: https://www.intel.com/programmable/technical-pdfs/683273.pdf Signed-off-by: Paweł Anikiel --- .../devicetree/bindings/media/intel,dprx.yaml | 160 ++ 1 file changed, 160 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml new file mode 100644 index ..31025f2d5dcd --- /dev/null +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml @@ -0,0 +1,160 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/intel,dprx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Intel DisplayPort RX IP + +maintainers: + - Paweł Anikiel + +description: | + The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP + Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video + capture and Multi-Stream Transport. + + The IP features a large number of configuration parameters, found at: + https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html + + The following parameters have to be enabled: +- Support DisplayPort sink +- Enable GPU control + The following parameters' values have to be set in the devicetree: +- RX maximum link rate +- Maximum lane count +- Support MST +- Max stream count (only if Support MST is enabled) + +properties: + compatible: +const: intel,dprx-20.0.1 + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + intel,max-link-rate: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Max link rate configuration parameter + + intel,max-lane-count: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Max lane count configuration parameter + + intel,multi-stream-support: +type: boolean +description: Multi-Stream Transport support configuration parameter + + intel,max-stream-count: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Max stream count configuration parameter + + port: +$ref: /schemas/graph.yaml#/properties/port +description: SST main link + + ports: +$ref: /schemas/graph.yaml#/properties/ports + +properties: + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: MST virtual channel 0 or SST main link + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: MST virtual channel 1 + + port@2: +$ref: /schemas/graph.yaml#/properties/port +description: MST virtual channel 2 + + port@3: +$ref: /schemas/graph.yaml#/properties/port +description: MST virtual channel 3 + +required: + - compatible + - reg + - interrupts + +allOf: + - if: + required: +- intel,multi-stream-support +then: + required: +- intel,max-stream-count +- ports +else: + required: +- port + +additionalProperties: false + +examples: + - | +#include + +dp-receiver@c0062000 { +compatible = "intel,dprx-20.0.1"; +reg = <0xc0062000 0x800>; +interrupt-parent = <_mst_irq>; +interrupts = <0 IRQ_TYPE_EDGE_RISING>; +intel,max-link-rate = <0x1e>; +intel,max-lane-count = <4>; +intel,multi-stream-support; +intel,max-stream-count = <4>; + +ports { +#address-cells = <1>; +#size-cells = <0>; + +port@0 { +reg = <0>; +dprx_mst_0: endpoint { +remote-endpoint = <_mst0_0>; +}; +}; + +port@1 { +reg = <1>; +dprx_mst_1: endpoint { +remote-endpoint = <_mst1_0>; +}; +}; + +port@2 { +reg = <2>; +dprx_mst_2: endpoint { +remote-endpoint = <_mst2_0>; +}; +}; + +port@3 { +reg = <3>; +dprx_mst_3: endpoint { +remote-endpoint = <_mst3_0>; +}; +}; +}; +}; + + - | +dp-receiver@c0064000 { +compatible = "intel,dprx-20.0.1"; +reg = <0xc0064000 0x800>; +interrupt-parent = <_sst_irq>; +interrupts = <0 IRQ_TYPE_EDGE_RISING>; +intel,max-link-rate = <0x1e>; +intel,max-lane-count = <4>; + +port { +dprx_sst_0: endpoint { +remote-endpoint = <_sst_0>; +}; +}; +}; -- 2.44.0.rc0.258.g7320e95886-goog
[PATCH v2 4/9] lib: Move DisplayPort CRC functions to common lib
The CRC functions found in drivers/gpu/drm/display/drm_dp_mst_topology.c may be useful for other non-DRM code that deals with DisplayPort, e.g. v4l2 drivers for DP receivers. Move these functions to /lib. Signed-off-by: Paweł Anikiel --- drivers/gpu/drm/display/Kconfig | 1 + drivers/gpu/drm/display/drm_dp_mst_topology.c | 76 ++ include/linux/crc-dp.h| 10 +++ lib/Kconfig | 8 ++ lib/Makefile | 1 + lib/crc-dp.c | 78 +++ 6 files changed, 103 insertions(+), 71 deletions(-) create mode 100644 include/linux/crc-dp.h create mode 100644 lib/crc-dp.c diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 09712b88a5b8..c615f50152f2 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -14,6 +14,7 @@ config DRM_DISPLAY_HELPER config DRM_DISPLAY_DP_HELPER bool depends on DRM_DISPLAY_HELPER + select CRC_DP help DRM display helpers for DisplayPort. diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index f7c6b60629c2..ada1f90fa808 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -195,73 +196,6 @@ drm_dp_mst_rad_to_str(const u8 rad[8], u8 lct, char *out, size_t len) } /* sideband msg handling */ -static u8 drm_dp_msg_header_crc4(const uint8_t *data, size_t num_nibbles) -{ - u8 bitmask = 0x80; - u8 bitshift = 7; - u8 array_index = 0; - int number_of_bits = num_nibbles * 4; - u8 remainder = 0; - - while (number_of_bits != 0) { - number_of_bits--; - remainder <<= 1; - remainder |= (data[array_index] & bitmask) >> bitshift; - bitmask >>= 1; - bitshift--; - if (bitmask == 0) { - bitmask = 0x80; - bitshift = 7; - array_index++; - } - if ((remainder & 0x10) == 0x10) - remainder ^= 0x13; - } - - number_of_bits = 4; - while (number_of_bits != 0) { - number_of_bits--; - remainder <<= 1; - if ((remainder & 0x10) != 0) - remainder ^= 0x13; - } - - return remainder; -} - -static u8 drm_dp_msg_data_crc4(const uint8_t *data, u8 number_of_bytes) -{ - u8 bitmask = 0x80; - u8 bitshift = 7; - u8 array_index = 0; - int number_of_bits = number_of_bytes * 8; - u16 remainder = 0; - - while (number_of_bits != 0) { - number_of_bits--; - remainder <<= 1; - remainder |= (data[array_index] & bitmask) >> bitshift; - bitmask >>= 1; - bitshift--; - if (bitmask == 0) { - bitmask = 0x80; - bitshift = 7; - array_index++; - } - if ((remainder & 0x100) == 0x100) - remainder ^= 0xd5; - } - - number_of_bits = 8; - while (number_of_bits != 0) { - number_of_bits--; - remainder <<= 1; - if ((remainder & 0x100) != 0) - remainder ^= 0xd5; - } - - return remainder & 0xff; -} static inline u8 drm_dp_calc_sb_hdr_size(struct drm_dp_sideband_msg_hdr *hdr) { u8 size = 3; @@ -284,7 +218,7 @@ static void drm_dp_encode_sideband_msg_hdr(struct drm_dp_sideband_msg_hdr *hdr, (hdr->msg_len & 0x3f); buf[idx++] = (hdr->somt << 7) | (hdr->eomt << 6) | (hdr->seqno << 4); - crc4 = drm_dp_msg_header_crc4(buf, (idx * 2) - 1); + crc4 = crc_dp_msg_header(buf, (idx * 2) - 1); buf[idx - 1] |= (crc4 & 0xf); *len = idx; @@ -305,7 +239,7 @@ static bool drm_dp_decode_sideband_msg_hdr(const struct drm_dp_mst_topology_mgr len += ((buf[0] & 0xf0) >> 4) / 2; if (len > buflen) return false; - crc4 = drm_dp_msg_header_crc4(buf, (len * 2) - 1); + crc4 = crc_dp_msg_header(buf, (len * 2) - 1); if ((crc4 & 0xf) != (buf[len - 1] & 0xf)) { drm_dbg_kms(mgr->dev, "crc4 mismatch 0x%x 0x%x\n", crc4, buf[len - 1]); @@ -725,7 +659,7 @@ static void drm_dp_crc_sideband_chunk_req(u8 *msg, u8 len) { u8 crc4; - crc4 = drm_dp_msg_data_crc4(msg, len); + crc4 = crc_dp_msg_data(msg, len); msg[len] = crc4; } @@ -782,7 +716,7 @@ static bool drm_dp_sideband_append_payload(struct drm_dp_sideband_msg_rx *msg, if (msg->curchunk_idx >= msg->curchunk_len) { /* do CRC */ -
[PATCH v2 9/9] ARM: dts: chameleonv3: Add video device nodes
Add device nodes for the video system present on the Chameleon v3. It consists of six framebuffers and two Intel Displayport receivers. Signed-off-by: Paweł Anikiel --- .../socfpga/socfpga_arria10_chameleonv3.dts | 152 ++ 1 file changed, 152 insertions(+) diff --git a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts index 422d00cd4c74..2f48f30cb538 100644 --- a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts +++ b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts @@ -10,6 +10,158 @@ / { compatible = "google,chameleon-v3", "enclustra,mercury-aa1", "altr,socfpga-arria10", "altr,socfpga"; + soc { + fb0: video@c0060500 { + compatible = "google,chv3-fb"; + reg = <0xc0060500 0x100>, + <0xc0060f20 0x10>; + interrupts = ; + google,legacy-format; + }; + + fb_mst0: video@c0060600 { + compatible = "google,chv3-fb"; + reg = <0xc0060600 0x100>, + <0xc0060f30 0x10>; + interrupts = ; + + port { + fb_mst0_0: endpoint { + remote-endpoint = <_mst_0>; + }; + }; + }; + + fb_mst1: video@c0060700 { + compatible = "google,chv3-fb"; + reg = <0xc0060700 0x100>, + <0xc0060f40 0x10>; + interrupts = ; + + port { + fb_mst1_0: endpoint { + remote-endpoint = <_mst_1>; + }; + }; + }; + + fb_mst2: video@c0060800 { + compatible = "google,chv3-fb"; + reg = <0xc0060800 0x100>, + <0xc0060f50 0x10>; + interrupts = ; + + port { + fb_mst2_0: endpoint { + remote-endpoint = <_mst_2>; + }; + }; + }; + + fb_mst3: video@c0060900 { + compatible = "google,chv3-fb"; + reg = <0xc0060900 0x100>, + <0xc0060f60 0x10>; + interrupts = ; + + port { + fb_mst3_0: endpoint { + remote-endpoint = <_mst_3>; + }; + }; + }; + + fb_sst: video@c0060a00 { + compatible = "google,chv3-fb"; + reg = <0xc0060a00 0x100>, + <0xc0060f70 0x10>; + interrupts = ; + + port { + fb_sst_0: endpoint { + remote-endpoint = <_sst_0>; + }; + }; + }; + + dprx_mst_irq: intc@c0060f80 { + compatible = "altr,pio-1.0"; + reg = <0xc0060f80 0x10>; + interrupts = ; + altr,interrupt-type = ; + #interrupt-cells = <2>; + interrupt-controller; + }; + + dprx_sst_irq: intc@c0060fe0 { + compatible = "altr,pio-1.0"; + reg = <0xc0060fe0 0x10>; + interrupts = ; + altr,interrupt-type = ; + #interrupt-cells = <2>; + interrupt-controller; + }; + + dprx_mst: dp-receiver@c0062000 { + compatible = "intel,dprx-20.0.1"; + reg = <0xc0062000 0x800>; + interrupt-parent = <_mst_irq>; + interrupts = <0 IRQ_TYPE_EDGE_RISING>; + intel,max-link-rate = <0x1e>; + intel,max-lane-count = <4>; + intel,multi-stream-support; + intel,max-stream-count = <4>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dprx_mst_0: endpoint { + remote-endpoint = <_mst0_0>; +
[PATCH v2 7/9] media: dt-bindings: Add Chameleon v3 framebuffer
The Chameleon v3 uses the framebuffer IP core to take the video signal from different sources and directly write frames into memory. Signed-off-by: Paweł Anikiel --- .../bindings/media/google,chv3-fb.yaml| 67 +++ 1 file changed, 67 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/google,chv3-fb.yaml diff --git a/Documentation/devicetree/bindings/media/google,chv3-fb.yaml b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml new file mode 100644 index ..7961c0ab66ec --- /dev/null +++ b/Documentation/devicetree/bindings/media/google,chv3-fb.yaml @@ -0,0 +1,67 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/google,chv3-fb.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Google Chameleon v3 video framebuffer + +maintainers: + - Paweł Anikiel + +properties: + compatible: +const: google,chv3-fb + + reg: +items: + - description: core registers + - description: irq registers + + interrupts: +maxItems: 1 + + google,legacy-format: +type: boolean +description: The incoming video stream is in 32-bit padded mode. + + port: +$ref: /schemas/graph.yaml#/properties/port +description: + Connection to the video receiver - optional. If this isn't present, + the video interface still works on its own, but EDID control is + unavailable and DV timing information only reports the active + video width/height. + +required: + - compatible + - reg + - interrupts + +additionalProperties: false + +examples: + - | +#include + +video@c0060500 { +compatible = "google,chv3-fb"; +reg = <0xc0060500 0x100>, + <0xc0060f20 0x10>; +interrupts = ; +google,legacy-format; +}; + + - | +video@c0060600 { +compatible = "google,chv3-fb"; +reg = <0xc0060600 0x100>, + <0xc0060f30 0x10>; +interrupts = ; + +port { +fb_mst0_0: endpoint { +remote-endpoint = <_mst_0>; +}; +}; +}; -- 2.44.0.rc0.258.g7320e95886-goog
[PATCH v2 5/9] drm/display: Add mask definitions for DP_PAYLOAD_ALLOCATE_* registers
Each of these registers contains a single value, but not the entire 8 bits: DP_PAYLOAD_ALLOCATE_SET - Bit 7 Reserved DP_PAYLOAD_ALLOCATE_START_TIME_SLOT - Bits 7:6 Reserved DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT - Bits 7:6 Reserved Add definitions to properly mask off values read from these registers. Signed-off-by: Paweł Anikiel --- include/drm/display/drm_dp.h | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 3731828825bd..9dee30190f14 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -733,8 +733,13 @@ # define DP_PANEL_REPLAY_SU_ENABLE (1 << 6) #define DP_PAYLOAD_ALLOCATE_SET0x1c0 -#define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1 -#define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2 +# define DP_PAYLOAD_ALLOCATE_SET_MASK 0x7f + +#define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT0x1c1 +# define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT_MASK 0x3f + +#define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT0x1c2 +# define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT_MASK 0x3f /* Link/Sink Device Status */ #define DP_SINK_COUNT 0x200 -- 2.44.0.rc0.258.g7320e95886-goog
[PATCH v2 6/9] media: intel: Add Displayport RX IP driver
Add driver for the Intel DisplayPort RX FPGA IP Signed-off-by: Paweł Anikiel --- drivers/media/platform/intel/Kconfig | 12 + drivers/media/platform/intel/Makefile |1 + drivers/media/platform/intel/intel-dprx.c | 2176 + 3 files changed, 2189 insertions(+) create mode 100644 drivers/media/platform/intel/intel-dprx.c diff --git a/drivers/media/platform/intel/Kconfig b/drivers/media/platform/intel/Kconfig index 724e80a9086d..eafcd47cce68 100644 --- a/drivers/media/platform/intel/Kconfig +++ b/drivers/media/platform/intel/Kconfig @@ -12,3 +12,15 @@ config VIDEO_PXA27x select V4L2_FWNODE help This is a v4l2 driver for the PXA27x Quick Capture Interface + +config VIDEO_INTEL_DPRX + tristate "Intel DisplayPort RX IP driver" + depends on V4L_PLATFORM_DRIVERS + depends on VIDEO_DEV + select V4L2_FWNODE + select CRC_DP + help + v4l2 subdev driver for Intel Displayport receiver FPGA IP. + It is a part of the DisplayPort Intel FPGA IP Core. + It implements a DisplayPort 1.4 receiver capable of HBR3 + video capture and Multi-Stream Transport. diff --git a/drivers/media/platform/intel/Makefile b/drivers/media/platform/intel/Makefile index 7e8889cbd2df..f571399f5aa8 100644 --- a/drivers/media/platform/intel/Makefile +++ b/drivers/media/platform/intel/Makefile @@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only obj-$(CONFIG_VIDEO_PXA27x) += pxa_camera.o +obj-$(CONFIG_VIDEO_INTEL_DPRX) += intel-dprx.o diff --git a/drivers/media/platform/intel/intel-dprx.c b/drivers/media/platform/intel/intel-dprx.c new file mode 100644 index ..d0c60e29e51d --- /dev/null +++ b/drivers/media/platform/intel/intel-dprx.c @@ -0,0 +1,2176 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2023-2024 Google LLC. + * Author: Paweł Anikiel + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DPRX_MAX_EDID_BLOCKS 4 + +/* DPRX registers */ + +#define DPRX_RX_CONTROL0x000 +#define DPRX_RX_CONTROL_LINK_RATE_SHIFT16 +#define DPRX_RX_CONTROL_LINK_RATE_MASK 0xff +#define DPRX_RX_CONTROL_RECONFIG_LINKRATE 13 +#define DPRX_RX_CONTROL_TP_SHIFT 8 +#define DPRX_RX_CONTROL_TP_MASK0x7 +#define DPRX_RX_CONTROL_SCRAMBLER_DISABLE 7 +#define DPRX_RX_CONTROL_CHANNEL_CODING_SHIFT 5 +#define DPRX_RX_CONTROL_CHANNEL_CODING_8B10B 0x1 +#define DPRX_RX_CONTROL_LANE_COUNT_SHIFT 0 +#define DPRX_RX_CONTROL_LANE_COUNT_MASK0x1f + +#define DPRX_RX_STATUS 0x001 +#define DPRX_RX_STATUS_INTERLANE_ALIGN 8 +#define DPRX_RX_STATUS_SYM_LOCK_SHIFT 4 +#define DPRX_RX_STATUS_SYM_LOCK(i) (4 + i) +#define DPRX_RX_STATUS_CR_LOCK_SHIFT 0 +#define DPRX_RX_STATUS_CR_LOCK(i) (0 + i) + +#define DPRX_MSA_HTOTAL(i) (0x022 + 0x20 * (i)) +#define DPRX_MSA_VTOTAL(i) (0x023 + 0x20 * (i)) +#define DPRX_MSA_HSP(i)(0x024 + 0x20 * (i)) +#define DPRX_MSA_HSW(i)(0x025 + 0x20 * (i)) +#define DPRX_MSA_HSTART(i) (0x026 + 0x20 * (i)) +#define DPRX_MSA_VSTART(i) (0x027 + 0x20 * (i)) +#define DPRX_MSA_VSP(i)(0x028 + 0x20 * (i)) +#define DPRX_MSA_VSW(i)(0x029 + 0x20 * (i)) +#define DPRX_MSA_HWIDTH(i) (0x02a + 0x20 * (i)) +#define DPRX_MSA_VHEIGHT(i)(0x02b + 0x20 * (i)) +#define DPRX_VBID(i) (0x02f + 0x20 * (i)) +#define DPRX_VBID_MSA_LOCK 7 + +#define DPRX_MST_CONTROL1 0x0a0 +#define DPRX_MST_CONTROL1_VCPTAB_UPD_FORCE 31 +#define DPRX_MST_CONTROL1_VCPTAB_UPD_REQ 30 +#define DPRX_MST_CONTROL1_VCP_ID_SHIFT(i) (4 + 4 * (i)) +#define DPRX_MST_CONTROL1_VCP_IDS_SHIFT4 +#define DPRX_MST_CONTROL1_VCP_IDS_MASK 0x +#define DPRX_MST_CONTROL1_MST_EN 0 + +#define DPRX_MST_STATUS1 0x0a1 +#define DPRX_MST_STATUS1_VCPTAB_ACT_ACK30 + +#define DPRX_MST_VCPTAB(i) (0x0a2 + i) + +#define DPRX_AUX_CONTROL 0x100 +#define DPRX_AUX_CONTROL_IRQ_EN8 +#define DPRX_AUX_CONTROL_TX_STROBE 7 +#define DPRX_AUX_CONTROL_LENGTH_SHIFT 0 +#define DPRX_AUX_CONTROL_LENGTH_MASK 0x1f + +#define DPRX_AUX_STATUS0x101 +#define DPRX_AUX_STATUS_MSG_READY 31 +#define DPRX_AUX_STATUS_READY_TO_TX30 + +#define DPRX_AUX_COMMAND 0x102 + +#define DPRX_AUX_HPD 0x119 +#define DPRX_AUX_HPD_IRQ 12