From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7149BC433EF for ; Wed, 13 Oct 2021 18:12:58 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 39F60610CF for ; Wed, 13 Oct 2021 18:12:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 39F60610CF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=LsUccXE4CYBkbjkooF4Tvw/DVgHJknDB8hdcpAN0lzk=; b=weH79fJrPW1CCP fF3AcrOKSH4kA5zaZrccdTPbpLdIpgPd7yOmJa8Rz4iqwb2eIRwpopOBOB3pLJcTXDq1NNGKaYkEC 4hGd2PyHbNA9wx3hCHOsQJYyg/SG4tZByIZBmaO99mYs1CIGbL4mJA7xSDSh69Y0oxcppLmUBcbnz virJyDysznUOQqHElhy34vN6+eWVO2igmoF+JwQDfyxb5bzwl2SENFERpJezKHEl+PH9uz77IlBjd TMcUYC9ZMQpaPu/EmmTQl66NImaeyqpwOKJdboV75ufjRN1WyBiasyD9sNT3c79U797VTkQ9Nvf6t TovzBhCAkT4fkyYN/cMw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1maik9-000GYv-GP; Wed, 13 Oct 2021 18:12:45 +0000 Received: from mail-qt1-x832.google.com ([2607:f8b0:4864:20::832]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1maik6-000GXw-HI for linux-mediatek@lists.infradead.org; Wed, 13 Oct 2021 18:12:44 +0000 Received: by mail-qt1-x832.google.com with SMTP id v17so3445120qtp.1 for ; Wed, 13 Oct 2021 11:12:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=BojWdLmJ832xQRuCmBl5mubNC5kYrEQRcd/BOgo7OG4cP5tYP5204U0XFBxdtLz24Y Hvdgj3UttPoRwbY3xaE7+Hp9+xzO7c47cziNkV7b8malqY/fLU8aUiXDxg89NccXUnP3 QgIAxk5Bjxaf+Stbm406fuQA2mz4meDQsEDz4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=W24TpO5xItJTu9iOJsNwUTj3jwok/gwv2kuxVvh2AP1GZLXn0riKKnO6ClBzWVBi1m yWleJAGNuNfCUwTmOi/fADW1wNm8NqPIKFbQVjvxw5cSpVLxtnDBaS3yUBfrSrZ1bNw7 a5qdQH2WDL+qgJ1JJeSZ1x7m10FA15KefKBnJAtT9DLTj6JK/XXRhPdg9Ii5jDxIGmTI /yBIkep5RELLjmiMZ6uII5EQNb0meED8BZitnJz4vAtCe64sEWd+CiajjFKsbcEZ5QYm lZEASJqhjsfW3YT3IeoP+eFnnwTwq2HkS4Ra+WCz1SN6mp7lxudCJd5/nHaWyeCYR0KN xdiA== X-Gm-Message-State: AOAM532UYn6TyxUxIaQmEtO83M3gTEmwuphAyN14cbVdsGuRXJPSDw25 90DvUhH5e7nayl+RlpxpIse3Bw== X-Google-Smtp-Source: ABdhPJxDWukRvC5VLi5XvI+Z7EFdxnKfGco6FCO6EkoXFnJKf2wn0AecstSiwkN5SFkY+6t1dXwqBg== X-Received: by 2002:a05:622a:170c:: with SMTP id h12mr106800qtk.6.1634148760232; Wed, 13 Oct 2021 11:12:40 -0700 (PDT) Received: from markyacoub.nyc.corp.google.com ([2620:15c:6c:200:61dd:96a:9268:3c4d]) by smtp.gmail.com with ESMTPSA id s203sm160130qke.21.2021.10.13.11.12.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Oct 2021 11:12:39 -0700 (PDT) From: Mark Yacoub To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: seanpaul@chromium.org, harry.wentland@amd.com, Mark Yacoub , Mark Yacoub , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Matthias Brugger , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Subject: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate. Date: Wed, 13 Oct 2021 14:12:20 -0400 Message-Id: <20211013181228.1578201-1-markyacoub@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20210929194012.3433306-1-markyacoub@chromium.org> References: <20210929194012.3433306-1-markyacoub@chromium.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211013_111242_671387_02757AB5 X-CRM114-Status: GOOD ( 25.34 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org From: Mark Yacoub [Why] 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma or Degamma props in the new CRTC state, allowing any invalid size to be passed on. 2. Each driver has its own LUT size, which could also be different for legacy users. [How] 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes assigned by the driver when it's initializing its color and CTM management. 2. Create drm_atomic_helper_check_crtc which is called by drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that they match the sizes in the new CRTC state. 3. Rename older lut checks that test for the color channels to indicate it's a channel check. It's not included in drm_atomic_helper_check_crtc as it's hardware specific and is to be called by the driver. 4. As the LUT size check now happens in drm_atomic_helper_check, remove the lut check in intel_color.c Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL) v1: 1. Fix typos 2. Remove the LUT size check from intel driver 3. Rename old LUT check to indicate it's a channel change Signed-off-by: Mark Yacoub --- drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++ drivers/gpu/drm/drm_color_mgmt.c | 14 ++--- drivers/gpu/drm/i915/display/intel_color.c | 14 ++--- include/drm/drm_atomic_helper.h | 1 + include/drm/drm_color_mgmt.h | 7 +-- include/drm/drm_crtc.h | 11 ++++ 6 files changed, 89 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index bc3487964fb5e..5feb2ad0209c3 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_helper_check_planes); +/** + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes + * @state: the driver state object + * + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new + * state holds them. + * + * RETURNS: + * Zero for success or -errno + */ +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *new_crtc_state; + int i; + + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) { + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->gamma_lut) { + uint64_t supported_lut_size = crtc->gamma_lut_size; + uint32_t supported_legacy_lut_size = crtc->gamma_size; + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->gamma_lut); + + if (new_state_lut_size != supported_lut_size && + new_state_lut_size != supported_legacy_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n", + supported_lut_size, + supported_legacy_lut_size, + new_state_lut_size); + return -EINVAL; + } + } + + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->degamma_lut) { + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->degamma_lut); + uint64_t supported_lut_size = crtc->degamma_lut_size; + + if (new_state_lut_size != supported_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Degamma LUT size. Should be %u but got %u.\n", + supported_lut_size, new_state_lut_size); + return -EINVAL; + } + } + } + + return 0; +} +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs); + /** * drm_atomic_helper_check - validate state object * @dev: DRM device @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev, if (ret) return ret; + ret = drm_atomic_helper_check_crtcs(state); + if (ret) + return ret; + if (state->legacy_cursor_update) state->async_update = !drm_atomic_helper_async_check(dev, state); diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6c..e5b820ce823bf 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, struct drm_mode_config *config = &dev->mode_config; if (degamma_lut_size) { + crtc->degamma_lut_size = degamma_lut_size; drm_object_attach_property(&crtc->base, config->degamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, config->ctm_property, 0); if (gamma_lut_size) { + crtc->gamma_lut_size = gamma_lut_size; drm_object_attach_property(&crtc->base, config->gamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane, EXPORT_SYMBOL(drm_plane_create_color_properties); /** - * drm_color_lut_check - check validity of lookup table + * drm_color_lut_channels_check - check validity of the channels in the lookup table * @lut: property blob containing LUT to check * @tests: bitmask of tests to run * - * Helper to check whether a userspace-provided lookup table is valid and - * satisfies hardware requirements. Drivers pass a bitmask indicating which of - * the tests in &drm_color_lut_tests should be performed. + * Helper to check whether each color channel of userspace-provided lookup table is valid and + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in + * &drm_color_lut_channels_tests should be performed. * * Returns 0 on success, -EINVAL on failure. */ -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests) { const struct drm_color_lut *entry; int i; @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) return 0; } -EXPORT_SYMBOL(drm_color_lut_check); +EXPORT_SYMBOL(drm_color_lut_channels_check); diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index dab892d2251ba..a308fe52746ac 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state) const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut; const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut; int gamma_length, degamma_length; - u32 gamma_tests, degamma_tests; + u32 gamma_channels_tests, degamma_channels_tests; /* Always allow legacy gamma LUT with no further checking. */ if (crtc_state_is_legacy_gamma(crtc_state)) @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state) degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size; gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size; - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; - if (check_lut_size(degamma_lut, degamma_length) || - check_lut_size(gamma_lut, gamma_length)) - return -EINVAL; - - if (drm_color_lut_check(degamma_lut, degamma_tests) || - drm_color_lut_check(gamma_lut, gamma_tests)) + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) || + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests)) return -EINVAL; return 0; diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 4045e2507e11c..a22d32a7a8719 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -38,6 +38,7 @@ struct drm_atomic_state; struct drm_private_obj; struct drm_private_state; +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state); int drm_atomic_helper_check_modeset(struct drm_device *dev, struct drm_atomic_state *state); int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index 81c298488b0c8..cb1bf361ad3e3 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane, enum drm_color_range default_range); /** - * enum drm_color_lut_tests - hw-specific LUT tests to perform + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform * * The drm_color_lut_check() function takes a bitmask of the values here to * determine which tests to apply to a userspace-provided LUT. */ -enum drm_color_lut_tests { +enum drm_color_lut_channels_tests { /** * @DRM_COLOR_LUT_EQUAL_CHANNELS: * @@ -119,5 +119,6 @@ enum drm_color_lut_tests { DRM_COLOR_LUT_NON_DECREASING = BIT(1), }; -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests); +int drm_color_lut_channels_check(const struct drm_property_blob *lut, + u32 tests); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 2deb15d7e1610..cabd3ef1a6e32 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1072,6 +1072,17 @@ struct drm_crtc { /** @funcs: CRTC control functions */ const struct drm_crtc_funcs *funcs; + /** + * @degamma_lut_size: Size of degamma LUT. + */ + uint32_t degamma_lut_size; + + /** + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as + * X, which doesn't support large lut sizes. + */ + uint32_t gamma_lut_size; + /** * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up * by calling drm_mode_crtc_set_gamma_size(). -- 2.33.0.882.g93a45727a2-goog _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6083FC433EF for ; Wed, 13 Oct 2021 18:13:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 48A25610CE for ; Wed, 13 Oct 2021 18:13:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233925AbhJMSPM (ORCPT ); Wed, 13 Oct 2021 14:15:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46082 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238606AbhJMSOo (ORCPT ); Wed, 13 Oct 2021 14:14:44 -0400 Received: from mail-qt1-x832.google.com (mail-qt1-x832.google.com [IPv6:2607:f8b0:4864:20::832]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1E622C061570 for ; Wed, 13 Oct 2021 11:12:41 -0700 (PDT) Received: by mail-qt1-x832.google.com with SMTP id w2so3488571qtn.0 for ; Wed, 13 Oct 2021 11:12:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=BojWdLmJ832xQRuCmBl5mubNC5kYrEQRcd/BOgo7OG4cP5tYP5204U0XFBxdtLz24Y Hvdgj3UttPoRwbY3xaE7+Hp9+xzO7c47cziNkV7b8malqY/fLU8aUiXDxg89NccXUnP3 QgIAxk5Bjxaf+Stbm406fuQA2mz4meDQsEDz4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=A+1KLFhqSkrHLQOdCd+Z2HU1jgLXjFbrIfzBG0C7Yr7xv0Kq6C5KLWfrvmtCniYygn Ax1meqXXrOMca5UXJHcHDBFTbYTpSQ882G8eVWpDaRO1bY7lCARTtAEdp0A3bllG+kyx 4VorUneSS7XRdL3YO+qOhxD8kXdpyWSyb30HifKsLz3TnpxukVrND8pEIO0VnkqXzwLW f2REAAVVTfN4FSOvANdrdvYc7CVtRYxWI9PrHGIJwcFoHvYt2GtIOdITF8YFSIUP4TYP J43a/ov+hj8d92JWUupQzLBPfo/7Nz3Fcdgf2v+K5iVfY/ohHe1gC8wnEl/VfvqHnIoo V/Qw== X-Gm-Message-State: AOAM530tgI9neyTQsEIaiZNbd6jxa5sQUs/4ZOnpjAwVyYYRdWgMDJLE Xw5LajSsl8azSAd0JmB2wErgbw== X-Google-Smtp-Source: ABdhPJxDWukRvC5VLi5XvI+Z7EFdxnKfGco6FCO6EkoXFnJKf2wn0AecstSiwkN5SFkY+6t1dXwqBg== X-Received: by 2002:a05:622a:170c:: with SMTP id h12mr106800qtk.6.1634148760232; Wed, 13 Oct 2021 11:12:40 -0700 (PDT) Received: from markyacoub.nyc.corp.google.com ([2620:15c:6c:200:61dd:96a:9268:3c4d]) by smtp.gmail.com with ESMTPSA id s203sm160130qke.21.2021.10.13.11.12.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Oct 2021 11:12:39 -0700 (PDT) From: Mark Yacoub To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: seanpaul@chromium.org, harry.wentland@amd.com, Mark Yacoub , Mark Yacoub , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Matthias Brugger , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Subject: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate. Date: Wed, 13 Oct 2021 14:12:20 -0400 Message-Id: <20211013181228.1578201-1-markyacoub@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20210929194012.3433306-1-markyacoub@chromium.org> References: <20210929194012.3433306-1-markyacoub@chromium.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Mark Yacoub [Why] 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma or Degamma props in the new CRTC state, allowing any invalid size to be passed on. 2. Each driver has its own LUT size, which could also be different for legacy users. [How] 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes assigned by the driver when it's initializing its color and CTM management. 2. Create drm_atomic_helper_check_crtc which is called by drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that they match the sizes in the new CRTC state. 3. Rename older lut checks that test for the color channels to indicate it's a channel check. It's not included in drm_atomic_helper_check_crtc as it's hardware specific and is to be called by the driver. 4. As the LUT size check now happens in drm_atomic_helper_check, remove the lut check in intel_color.c Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL) v1: 1. Fix typos 2. Remove the LUT size check from intel driver 3. Rename old LUT check to indicate it's a channel change Signed-off-by: Mark Yacoub --- drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++ drivers/gpu/drm/drm_color_mgmt.c | 14 ++--- drivers/gpu/drm/i915/display/intel_color.c | 14 ++--- include/drm/drm_atomic_helper.h | 1 + include/drm/drm_color_mgmt.h | 7 +-- include/drm/drm_crtc.h | 11 ++++ 6 files changed, 89 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index bc3487964fb5e..5feb2ad0209c3 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_helper_check_planes); +/** + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes + * @state: the driver state object + * + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new + * state holds them. + * + * RETURNS: + * Zero for success or -errno + */ +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *new_crtc_state; + int i; + + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) { + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->gamma_lut) { + uint64_t supported_lut_size = crtc->gamma_lut_size; + uint32_t supported_legacy_lut_size = crtc->gamma_size; + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->gamma_lut); + + if (new_state_lut_size != supported_lut_size && + new_state_lut_size != supported_legacy_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n", + supported_lut_size, + supported_legacy_lut_size, + new_state_lut_size); + return -EINVAL; + } + } + + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->degamma_lut) { + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->degamma_lut); + uint64_t supported_lut_size = crtc->degamma_lut_size; + + if (new_state_lut_size != supported_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Degamma LUT size. Should be %u but got %u.\n", + supported_lut_size, new_state_lut_size); + return -EINVAL; + } + } + } + + return 0; +} +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs); + /** * drm_atomic_helper_check - validate state object * @dev: DRM device @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev, if (ret) return ret; + ret = drm_atomic_helper_check_crtcs(state); + if (ret) + return ret; + if (state->legacy_cursor_update) state->async_update = !drm_atomic_helper_async_check(dev, state); diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6c..e5b820ce823bf 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, struct drm_mode_config *config = &dev->mode_config; if (degamma_lut_size) { + crtc->degamma_lut_size = degamma_lut_size; drm_object_attach_property(&crtc->base, config->degamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, config->ctm_property, 0); if (gamma_lut_size) { + crtc->gamma_lut_size = gamma_lut_size; drm_object_attach_property(&crtc->base, config->gamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane, EXPORT_SYMBOL(drm_plane_create_color_properties); /** - * drm_color_lut_check - check validity of lookup table + * drm_color_lut_channels_check - check validity of the channels in the lookup table * @lut: property blob containing LUT to check * @tests: bitmask of tests to run * - * Helper to check whether a userspace-provided lookup table is valid and - * satisfies hardware requirements. Drivers pass a bitmask indicating which of - * the tests in &drm_color_lut_tests should be performed. + * Helper to check whether each color channel of userspace-provided lookup table is valid and + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in + * &drm_color_lut_channels_tests should be performed. * * Returns 0 on success, -EINVAL on failure. */ -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests) { const struct drm_color_lut *entry; int i; @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) return 0; } -EXPORT_SYMBOL(drm_color_lut_check); +EXPORT_SYMBOL(drm_color_lut_channels_check); diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index dab892d2251ba..a308fe52746ac 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state) const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut; const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut; int gamma_length, degamma_length; - u32 gamma_tests, degamma_tests; + u32 gamma_channels_tests, degamma_channels_tests; /* Always allow legacy gamma LUT with no further checking. */ if (crtc_state_is_legacy_gamma(crtc_state)) @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state) degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size; gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size; - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; - if (check_lut_size(degamma_lut, degamma_length) || - check_lut_size(gamma_lut, gamma_length)) - return -EINVAL; - - if (drm_color_lut_check(degamma_lut, degamma_tests) || - drm_color_lut_check(gamma_lut, gamma_tests)) + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) || + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests)) return -EINVAL; return 0; diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 4045e2507e11c..a22d32a7a8719 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -38,6 +38,7 @@ struct drm_atomic_state; struct drm_private_obj; struct drm_private_state; +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state); int drm_atomic_helper_check_modeset(struct drm_device *dev, struct drm_atomic_state *state); int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index 81c298488b0c8..cb1bf361ad3e3 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane, enum drm_color_range default_range); /** - * enum drm_color_lut_tests - hw-specific LUT tests to perform + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform * * The drm_color_lut_check() function takes a bitmask of the values here to * determine which tests to apply to a userspace-provided LUT. */ -enum drm_color_lut_tests { +enum drm_color_lut_channels_tests { /** * @DRM_COLOR_LUT_EQUAL_CHANNELS: * @@ -119,5 +119,6 @@ enum drm_color_lut_tests { DRM_COLOR_LUT_NON_DECREASING = BIT(1), }; -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests); +int drm_color_lut_channels_check(const struct drm_property_blob *lut, + u32 tests); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 2deb15d7e1610..cabd3ef1a6e32 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1072,6 +1072,17 @@ struct drm_crtc { /** @funcs: CRTC control functions */ const struct drm_crtc_funcs *funcs; + /** + * @degamma_lut_size: Size of degamma LUT. + */ + uint32_t degamma_lut_size; + + /** + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as + * X, which doesn't support large lut sizes. + */ + uint32_t gamma_lut_size; + /** * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up * by calling drm_mode_crtc_set_gamma_size(). -- 2.33.0.882.g93a45727a2-goog From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1525C433F5 for ; Wed, 13 Oct 2021 18:12:43 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6E14A610CF for ; Wed, 13 Oct 2021 18:12:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6E14A610CF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 989EB6E105; Wed, 13 Oct 2021 18:12:42 +0000 (UTC) Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3FE7689EEB for ; Wed, 13 Oct 2021 18:12:41 +0000 (UTC) Received: by mail-qt1-x82d.google.com with SMTP id w2so3488576qtn.0 for ; Wed, 13 Oct 2021 11:12:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=BojWdLmJ832xQRuCmBl5mubNC5kYrEQRcd/BOgo7OG4cP5tYP5204U0XFBxdtLz24Y Hvdgj3UttPoRwbY3xaE7+Hp9+xzO7c47cziNkV7b8malqY/fLU8aUiXDxg89NccXUnP3 QgIAxk5Bjxaf+Stbm406fuQA2mz4meDQsEDz4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=mJYra/ym+hCsenjApmV94Tq/cf4N5sl187VkpZOrWD0b8+0hC3ynmcNeUpCaJeH/5t EQU4Iit+tc3w78NiNIZIbzW+ttFC7uEnX/VbgLwWE0x8L5+y634Y9ObpR4o1aFDEbFOW Q62GZNthzft6dzR4ix1+EyP/GEnTLxU/M1Yy3GrzdegLE4QoSi3GbCNzzqbSqOtw2CH4 EUgm4gtOmm+nQnC1i3Dvc1zm7xJnnHhtMasKOHRi6Ggfqy8lZIqMBy0r8GQn2VAsUvnc GlTwrFFtfLss0RaSQ+EzTGhu2wy1vwTp5RLtX/BLbHXjx/EBOKGZ9dD5Vj5BcGdV68MA RyKQ== X-Gm-Message-State: AOAM530pEbZYHzJjR2p8ny/eXIp8IKnAOcAMwa1b8DuFmuQpLuvNXb+n E8gU1S4+EX8JctdezdgvthlIaw== X-Google-Smtp-Source: ABdhPJxDWukRvC5VLi5XvI+Z7EFdxnKfGco6FCO6EkoXFnJKf2wn0AecstSiwkN5SFkY+6t1dXwqBg== X-Received: by 2002:a05:622a:170c:: with SMTP id h12mr106800qtk.6.1634148760232; Wed, 13 Oct 2021 11:12:40 -0700 (PDT) Received: from markyacoub.nyc.corp.google.com ([2620:15c:6c:200:61dd:96a:9268:3c4d]) by smtp.gmail.com with ESMTPSA id s203sm160130qke.21.2021.10.13.11.12.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Oct 2021 11:12:39 -0700 (PDT) From: Mark Yacoub To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: seanpaul@chromium.org, harry.wentland@amd.com, Mark Yacoub , Mark Yacoub , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Matthias Brugger , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Date: Wed, 13 Oct 2021 14:12:20 -0400 Message-Id: <20211013181228.1578201-1-markyacoub@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20210929194012.3433306-1-markyacoub@chromium.org> References: <20210929194012.3433306-1-markyacoub@chromium.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Intel-gfx] [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: Mark Yacoub [Why] 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma or Degamma props in the new CRTC state, allowing any invalid size to be passed on. 2. Each driver has its own LUT size, which could also be different for legacy users. [How] 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes assigned by the driver when it's initializing its color and CTM management. 2. Create drm_atomic_helper_check_crtc which is called by drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that they match the sizes in the new CRTC state. 3. Rename older lut checks that test for the color channels to indicate it's a channel check. It's not included in drm_atomic_helper_check_crtc as it's hardware specific and is to be called by the driver. 4. As the LUT size check now happens in drm_atomic_helper_check, remove the lut check in intel_color.c Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL) v1: 1. Fix typos 2. Remove the LUT size check from intel driver 3. Rename old LUT check to indicate it's a channel change Signed-off-by: Mark Yacoub --- drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++ drivers/gpu/drm/drm_color_mgmt.c | 14 ++--- drivers/gpu/drm/i915/display/intel_color.c | 14 ++--- include/drm/drm_atomic_helper.h | 1 + include/drm/drm_color_mgmt.h | 7 +-- include/drm/drm_crtc.h | 11 ++++ 6 files changed, 89 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index bc3487964fb5e..5feb2ad0209c3 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_helper_check_planes); +/** + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes + * @state: the driver state object + * + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new + * state holds them. + * + * RETURNS: + * Zero for success or -errno + */ +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *new_crtc_state; + int i; + + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) { + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->gamma_lut) { + uint64_t supported_lut_size = crtc->gamma_lut_size; + uint32_t supported_legacy_lut_size = crtc->gamma_size; + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->gamma_lut); + + if (new_state_lut_size != supported_lut_size && + new_state_lut_size != supported_legacy_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n", + supported_lut_size, + supported_legacy_lut_size, + new_state_lut_size); + return -EINVAL; + } + } + + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->degamma_lut) { + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->degamma_lut); + uint64_t supported_lut_size = crtc->degamma_lut_size; + + if (new_state_lut_size != supported_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Degamma LUT size. Should be %u but got %u.\n", + supported_lut_size, new_state_lut_size); + return -EINVAL; + } + } + } + + return 0; +} +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs); + /** * drm_atomic_helper_check - validate state object * @dev: DRM device @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev, if (ret) return ret; + ret = drm_atomic_helper_check_crtcs(state); + if (ret) + return ret; + if (state->legacy_cursor_update) state->async_update = !drm_atomic_helper_async_check(dev, state); diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6c..e5b820ce823bf 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, struct drm_mode_config *config = &dev->mode_config; if (degamma_lut_size) { + crtc->degamma_lut_size = degamma_lut_size; drm_object_attach_property(&crtc->base, config->degamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, config->ctm_property, 0); if (gamma_lut_size) { + crtc->gamma_lut_size = gamma_lut_size; drm_object_attach_property(&crtc->base, config->gamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane, EXPORT_SYMBOL(drm_plane_create_color_properties); /** - * drm_color_lut_check - check validity of lookup table + * drm_color_lut_channels_check - check validity of the channels in the lookup table * @lut: property blob containing LUT to check * @tests: bitmask of tests to run * - * Helper to check whether a userspace-provided lookup table is valid and - * satisfies hardware requirements. Drivers pass a bitmask indicating which of - * the tests in &drm_color_lut_tests should be performed. + * Helper to check whether each color channel of userspace-provided lookup table is valid and + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in + * &drm_color_lut_channels_tests should be performed. * * Returns 0 on success, -EINVAL on failure. */ -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests) { const struct drm_color_lut *entry; int i; @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) return 0; } -EXPORT_SYMBOL(drm_color_lut_check); +EXPORT_SYMBOL(drm_color_lut_channels_check); diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index dab892d2251ba..a308fe52746ac 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state) const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut; const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut; int gamma_length, degamma_length; - u32 gamma_tests, degamma_tests; + u32 gamma_channels_tests, degamma_channels_tests; /* Always allow legacy gamma LUT with no further checking. */ if (crtc_state_is_legacy_gamma(crtc_state)) @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state) degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size; gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size; - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; - if (check_lut_size(degamma_lut, degamma_length) || - check_lut_size(gamma_lut, gamma_length)) - return -EINVAL; - - if (drm_color_lut_check(degamma_lut, degamma_tests) || - drm_color_lut_check(gamma_lut, gamma_tests)) + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) || + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests)) return -EINVAL; return 0; diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 4045e2507e11c..a22d32a7a8719 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -38,6 +38,7 @@ struct drm_atomic_state; struct drm_private_obj; struct drm_private_state; +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state); int drm_atomic_helper_check_modeset(struct drm_device *dev, struct drm_atomic_state *state); int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index 81c298488b0c8..cb1bf361ad3e3 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane, enum drm_color_range default_range); /** - * enum drm_color_lut_tests - hw-specific LUT tests to perform + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform * * The drm_color_lut_check() function takes a bitmask of the values here to * determine which tests to apply to a userspace-provided LUT. */ -enum drm_color_lut_tests { +enum drm_color_lut_channels_tests { /** * @DRM_COLOR_LUT_EQUAL_CHANNELS: * @@ -119,5 +119,6 @@ enum drm_color_lut_tests { DRM_COLOR_LUT_NON_DECREASING = BIT(1), }; -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests); +int drm_color_lut_channels_check(const struct drm_property_blob *lut, + u32 tests); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 2deb15d7e1610..cabd3ef1a6e32 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1072,6 +1072,17 @@ struct drm_crtc { /** @funcs: CRTC control functions */ const struct drm_crtc_funcs *funcs; + /** + * @degamma_lut_size: Size of degamma LUT. + */ + uint32_t degamma_lut_size; + + /** + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as + * X, which doesn't support large lut sizes. + */ + uint32_t gamma_lut_size; + /** * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up * by calling drm_mode_crtc_set_gamma_size(). -- 2.33.0.882.g93a45727a2-goog From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C8341C433F5 for ; Wed, 13 Oct 2021 18:14:51 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 951CE61130 for ; Wed, 13 Oct 2021 18:14:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 951CE61130 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=byD6RDohyQoIYWMLco8O9xpWzzXLkE+rTWZ/ZCqgvBQ=; b=1Rf2vNItkUSpFK DjRvXx55udCAaH2walnvM4oOvGKy31AL1ht5iJ5qknT0WaGs/WxE/aM7ET4wJFxCuv6ueA9R/8g1b PGK45uDCbRzHZMYZ8nAN2zjwf74Qdbn8+m8P8xH9FOajLmpaR9hER3ZQE15YyXyCE80cOlxpSfZ13 yHeZog93NIr6HqzerJFJdmoqQ/j2519VxwvSaPD+3SplWpzBwgQl7dF0r9dt5UtcHH830W0beXAiz 6Zg0lBMDrUa6XcU11jxRlo+gnGw+c97qKUlJ5blZunxOIj+KGwe+5uXXv0Q+/XdbUkSZ9kMjldNC4 PKudwc8tglm9FM+aZ0eQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1maikB-000GZ1-Bd; Wed, 13 Oct 2021 18:12:47 +0000 Received: from mail-qt1-x82c.google.com ([2607:f8b0:4864:20::82c]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1maik6-000GXx-Le for linux-arm-kernel@lists.infradead.org; Wed, 13 Oct 2021 18:12:44 +0000 Received: by mail-qt1-x82c.google.com with SMTP id z24so3402603qtv.9 for ; Wed, 13 Oct 2021 11:12:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=BojWdLmJ832xQRuCmBl5mubNC5kYrEQRcd/BOgo7OG4cP5tYP5204U0XFBxdtLz24Y Hvdgj3UttPoRwbY3xaE7+Hp9+xzO7c47cziNkV7b8malqY/fLU8aUiXDxg89NccXUnP3 QgIAxk5Bjxaf+Stbm406fuQA2mz4meDQsEDz4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=d6D5/IzB04/sRINqDPyMaeFU8sU1rQ5pGr4cDho+SKQ=; b=hsIvw9OjLVVaAr4GyFtSrhX3Ocsb9Y572jx6iwW3wMC6ljLlnbjGQ5pV3N0hf4cA4h 2a/j7qng9+e4ycNWzfjAOW4DtgP+z17vHr+dYW901iiipS0Mmjk0akhDbuQvNjrwAl0j jC7zfAd58/9ii6cA8Lbh7MAAai6AlkiRraIVU+CCrpxhe5/VbM7b9omYWFLBt4ge61GM tTgPC03IudaqmlEUqnpEXs2KqiO67eV/wxiCusLplGD/eAPmSvZH4gLXguUhaRDkrf4+ 3r0978NVQnojHrqUaJwievy7iGbSYx0tat4vh9i73pDXCc3hqSKrjOV+TiwW1B8CbCDx 3XTA== X-Gm-Message-State: AOAM532g5NriJxEHDK9Y9svt3cIJbcLmiSRf5javsBNUl6DGUOZe3wGa OO1/GjHi7Fe3Xx1sl70sVmiNRw== X-Google-Smtp-Source: ABdhPJxDWukRvC5VLi5XvI+Z7EFdxnKfGco6FCO6EkoXFnJKf2wn0AecstSiwkN5SFkY+6t1dXwqBg== X-Received: by 2002:a05:622a:170c:: with SMTP id h12mr106800qtk.6.1634148760232; Wed, 13 Oct 2021 11:12:40 -0700 (PDT) Received: from markyacoub.nyc.corp.google.com ([2620:15c:6c:200:61dd:96a:9268:3c4d]) by smtp.gmail.com with ESMTPSA id s203sm160130qke.21.2021.10.13.11.12.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Oct 2021 11:12:39 -0700 (PDT) From: Mark Yacoub To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: seanpaul@chromium.org, harry.wentland@amd.com, Mark Yacoub , Mark Yacoub , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Matthias Brugger , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Subject: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate. Date: Wed, 13 Oct 2021 14:12:20 -0400 Message-Id: <20211013181228.1578201-1-markyacoub@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20210929194012.3433306-1-markyacoub@chromium.org> References: <20210929194012.3433306-1-markyacoub@chromium.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211013_111242_719260_C97BDCF4 X-CRM114-Status: GOOD ( 26.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Mark Yacoub [Why] 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma or Degamma props in the new CRTC state, allowing any invalid size to be passed on. 2. Each driver has its own LUT size, which could also be different for legacy users. [How] 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes assigned by the driver when it's initializing its color and CTM management. 2. Create drm_atomic_helper_check_crtc which is called by drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that they match the sizes in the new CRTC state. 3. Rename older lut checks that test for the color channels to indicate it's a channel check. It's not included in drm_atomic_helper_check_crtc as it's hardware specific and is to be called by the driver. 4. As the LUT size check now happens in drm_atomic_helper_check, remove the lut check in intel_color.c Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL) v1: 1. Fix typos 2. Remove the LUT size check from intel driver 3. Rename old LUT check to indicate it's a channel change Signed-off-by: Mark Yacoub --- drivers/gpu/drm/drm_atomic_helper.c | 60 ++++++++++++++++++++++ drivers/gpu/drm/drm_color_mgmt.c | 14 ++--- drivers/gpu/drm/i915/display/intel_color.c | 14 ++--- include/drm/drm_atomic_helper.h | 1 + include/drm/drm_color_mgmt.h | 7 +-- include/drm/drm_crtc.h | 11 ++++ 6 files changed, 89 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index bc3487964fb5e..5feb2ad0209c3 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_helper_check_planes); +/** + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes + * @state: the driver state object + * + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new + * state holds them. + * + * RETURNS: + * Zero for success or -errno + */ +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *new_crtc_state; + int i; + + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) { + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->gamma_lut) { + uint64_t supported_lut_size = crtc->gamma_lut_size; + uint32_t supported_legacy_lut_size = crtc->gamma_size; + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->gamma_lut); + + if (new_state_lut_size != supported_lut_size && + new_state_lut_size != supported_legacy_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Gamma LUT size. Should be %u (or %u for legacy) but got %u.\n", + supported_lut_size, + supported_legacy_lut_size, + new_state_lut_size); + return -EINVAL; + } + } + + if (new_crtc_state->color_mgmt_changed && + new_crtc_state->degamma_lut) { + uint32_t new_state_lut_size = + drm_color_lut_size(new_crtc_state->degamma_lut); + uint64_t supported_lut_size = crtc->degamma_lut_size; + + if (new_state_lut_size != supported_lut_size) { + drm_dbg_state( + state->dev, + "Invalid Degamma LUT size. Should be %u but got %u.\n", + supported_lut_size, new_state_lut_size); + return -EINVAL; + } + } + } + + return 0; +} +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs); + /** * drm_atomic_helper_check - validate state object * @dev: DRM device @@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev, if (ret) return ret; + ret = drm_atomic_helper_check_crtcs(state); + if (ret) + return ret; + if (state->legacy_cursor_update) state->async_update = !drm_atomic_helper_async_check(dev, state); diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6c..e5b820ce823bf 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, struct drm_mode_config *config = &dev->mode_config; if (degamma_lut_size) { + crtc->degamma_lut_size = degamma_lut_size; drm_object_attach_property(&crtc->base, config->degamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -178,6 +179,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, config->ctm_property, 0); if (gamma_lut_size) { + crtc->gamma_lut_size = gamma_lut_size; drm_object_attach_property(&crtc->base, config->gamma_lut_property, 0); drm_object_attach_property(&crtc->base, @@ -585,17 +587,17 @@ int drm_plane_create_color_properties(struct drm_plane *plane, EXPORT_SYMBOL(drm_plane_create_color_properties); /** - * drm_color_lut_check - check validity of lookup table + * drm_color_lut_channels_check - check validity of the channels in the lookup table * @lut: property blob containing LUT to check * @tests: bitmask of tests to run * - * Helper to check whether a userspace-provided lookup table is valid and - * satisfies hardware requirements. Drivers pass a bitmask indicating which of - * the tests in &drm_color_lut_tests should be performed. + * Helper to check whether each color channel of userspace-provided lookup table is valid and + * satisfies hardware requirements. Drivers pass a bitmask indicating which of in + * &drm_color_lut_channels_tests should be performed. * * Returns 0 on success, -EINVAL on failure. */ -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) +int drm_color_lut_channels_check(const struct drm_property_blob *lut, u32 tests) { const struct drm_color_lut *entry; int i; @@ -625,4 +627,4 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests) return 0; } -EXPORT_SYMBOL(drm_color_lut_check); +EXPORT_SYMBOL(drm_color_lut_channels_check); diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index dab892d2251ba..a308fe52746ac 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1285,7 +1285,7 @@ static int check_luts(const struct intel_crtc_state *crtc_state) const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut; const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut; int gamma_length, degamma_length; - u32 gamma_tests, degamma_tests; + u32 gamma_channels_tests, degamma_channels_tests; /* Always allow legacy gamma LUT with no further checking. */ if (crtc_state_is_legacy_gamma(crtc_state)) @@ -1300,15 +1300,11 @@ static int check_luts(const struct intel_crtc_state *crtc_state) degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size; gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size; - degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; - gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; + degamma_channels_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests; + gamma_channels_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests; - if (check_lut_size(degamma_lut, degamma_length) || - check_lut_size(gamma_lut, gamma_length)) - return -EINVAL; - - if (drm_color_lut_check(degamma_lut, degamma_tests) || - drm_color_lut_check(gamma_lut, gamma_tests)) + if (drm_color_lut_channels_check(degamma_lut, degamma_channels_tests) || + drm_color_lut_channels_check(gamma_lut, gamma_channels_tests)) return -EINVAL; return 0; diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 4045e2507e11c..a22d32a7a8719 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -38,6 +38,7 @@ struct drm_atomic_state; struct drm_private_obj; struct drm_private_state; +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state); int drm_atomic_helper_check_modeset(struct drm_device *dev, struct drm_atomic_state *state); int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index 81c298488b0c8..cb1bf361ad3e3 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -94,12 +94,12 @@ int drm_plane_create_color_properties(struct drm_plane *plane, enum drm_color_range default_range); /** - * enum drm_color_lut_tests - hw-specific LUT tests to perform + * enum drm_color_lut_channels_tests - hw-specific LUT tests to perform * * The drm_color_lut_check() function takes a bitmask of the values here to * determine which tests to apply to a userspace-provided LUT. */ -enum drm_color_lut_tests { +enum drm_color_lut_channels_tests { /** * @DRM_COLOR_LUT_EQUAL_CHANNELS: * @@ -119,5 +119,6 @@ enum drm_color_lut_tests { DRM_COLOR_LUT_NON_DECREASING = BIT(1), }; -int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests); +int drm_color_lut_channels_check(const struct drm_property_blob *lut, + u32 tests); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 2deb15d7e1610..cabd3ef1a6e32 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1072,6 +1072,17 @@ struct drm_crtc { /** @funcs: CRTC control functions */ const struct drm_crtc_funcs *funcs; + /** + * @degamma_lut_size: Size of degamma LUT. + */ + uint32_t degamma_lut_size; + + /** + * @gamma_lut_size: Size of Gamma LUT. Not used by legacy userspace such as + * X, which doesn't support large lut sizes. + */ + uint32_t gamma_lut_size; + /** * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up * by calling drm_mode_crtc_set_gamma_size(). -- 2.33.0.882.g93a45727a2-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel