[PATCH v1 0/2] drm/komeda: refactor sysfs node "config_id"

[PATCH v1 0/2] drm/komeda: refactor sysfs node "config_id"

[james qian wang (Arm Technology China)]

Split sysfs node "config_id" to multiple files.

James Qian Wang (Arm Technology China) (2):
  drm/komeda: Add a new file komeda_sysfs.c
  drm/komeda: Refactor sysfs node "config_id"

 .../drm/arm/display/include/malidp_product.h  |  13 --
 drivers/gpu/drm/arm/display/komeda/Makefile   |   1 +
 .../gpu/drm/arm/display/komeda/komeda_dev.c   |  61 +--------
 .../gpu/drm/arm/display/komeda/komeda_dev.h   |   3 +
 .../gpu/drm/arm/display/komeda/komeda_sysfs.c | 125 ++++++++++++++++++
 5 files changed, 132 insertions(+), 71 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c

--
2.20.1

[PATCH v1 1/2] drm/komeda: Add a new file komeda_sysfs.c

[james qian wang (Arm Technology China)]

From: "James Qian Wang (Arm Technology China)" <james.qian.wang@arm.com>

Add a new file komeda_sysfs.c and move all sysfs related code to it.

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 drivers/gpu/drm/arm/display/komeda/Makefile   |  1 +
 .../gpu/drm/arm/display/komeda/komeda_dev.c   | 61 +-------------
 .../gpu/drm/arm/display/komeda/komeda_dev.h   |  3 +
 .../gpu/drm/arm/display/komeda/komeda_sysfs.c | 81 +++++++++++++++++++
 4 files changed, 88 insertions(+), 58 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c

diff --git a/drivers/gpu/drm/arm/display/komeda/Makefile b/drivers/gpu/drm/arm/display/komeda/Makefile
index 1931a7fa1a14..706674ca5928 100644
--- a/drivers/gpu/drm/arm/display/komeda/Makefile
+++ b/drivers/gpu/drm/arm/display/komeda/Makefile
@@ -7,6 +7,7 @@ ccflags-y := \
 komeda-y := \
 	komeda_drv.o \
 	komeda_dev.o \
+	komeda_sysfs.o \
 	komeda_format_caps.o \
 	komeda_color_mgmt.o \
 	komeda_pipeline.o \
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index 8e0bce46555b..734b88b88d94 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -63,59 +63,6 @@ static void komeda_debugfs_init(struct komeda_dev *mdev)
 }
 #endif
 
-static ssize_t
-core_id_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct komeda_dev *mdev = dev_to_mdev(dev);
-
-	return snprintf(buf, PAGE_SIZE, "0x%08x\n", mdev->chip.core_id);
-}
-static DEVICE_ATTR_RO(core_id);
-
-static ssize_t
-config_id_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct komeda_dev *mdev = dev_to_mdev(dev);
-	struct komeda_pipeline *pipe = mdev->pipelines[0];
-	union komeda_config_id config_id;
-	int i;
-
-	memset(&config_id, 0, sizeof(config_id));
-
-	config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
-	config_id.side_by_side = mdev->side_by_side;
-	config_id.n_pipelines = mdev->n_pipelines;
-	config_id.n_scalers = pipe->n_scalers;
-	config_id.n_layers = pipe->n_layers;
-	config_id.n_richs = 0;
-	for (i = 0; i < pipe->n_layers; i++) {
-		if (pipe->layers[i]->layer_type == KOMEDA_FMT_RICH_LAYER)
-			config_id.n_richs++;
-	}
-	return snprintf(buf, PAGE_SIZE, "0x%08x\n", config_id.value);
-}
-static DEVICE_ATTR_RO(config_id);
-
-static ssize_t
-aclk_hz_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct komeda_dev *mdev = dev_to_mdev(dev);
-
-	return snprintf(buf, PAGE_SIZE, "%lu\n", clk_get_rate(mdev->aclk));
-}
-static DEVICE_ATTR_RO(aclk_hz);
-
-static struct attribute *komeda_sysfs_entries[] = {
-	&dev_attr_core_id.attr,
-	&dev_attr_config_id.attr,
-	&dev_attr_aclk_hz.attr,
-	NULL,
-};
-
-static struct attribute_group komeda_sysfs_attr_group = {
-	.attrs = komeda_sysfs_entries,
-};
-
 static int komeda_parse_pipe_dt(struct komeda_pipeline *pipe)
 {
 	struct device_node *np = pipe->of_node;
@@ -277,11 +224,9 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
 
 	clk_disable_unprepare(mdev->aclk);
 
-	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
-	if (err) {
-		DRM_ERROR("create sysfs group failed.\n");
+	err = komeda_dev_sysfs_init(mdev);
+	if (err)
 		goto err_cleanup;
-	}
 
 	mdev->err_verbosity = KOMEDA_DEV_PRINT_ERR_EVENTS;
 
@@ -304,7 +249,7 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
 	const struct komeda_dev_funcs *funcs = mdev->funcs;
 	int i;
 
-	sysfs_remove_group(&dev->kobj, &komeda_sysfs_attr_group);
+	komeda_dev_sysfs_destroy(mdev);
 
 #ifdef CONFIG_DEBUG_FS
 	debugfs_remove_recursive(mdev->debugfs_root);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index dacdb00153e9..6183e0f394f0 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -248,4 +248,7 @@ void komeda_print_events(struct komeda_events *evts, struct drm_device *dev);
 int komeda_dev_resume(struct komeda_dev *mdev);
 int komeda_dev_suspend(struct komeda_dev *mdev);
 
+int komeda_dev_sysfs_init(struct komeda_dev *mdev);
+void komeda_dev_sysfs_destroy(struct komeda_dev *mdev);
+
 #endif /*_KOMEDA_DEV_H_*/
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
new file mode 100644
index 000000000000..740f095b4ca5
--- /dev/null
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) COPYRIGHT 2019 ARM Limited. All rights reserved.
+ * Author: James.Qian.Wang <james.qian.wang@arm.com>
+ *
+ */
+#include <drm/drm_print.h>
+
+#include "komeda_dev.h"
+
+static ssize_t
+core_id_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+
+	return snprintf(buf, PAGE_SIZE, "0x%08x\n", mdev->chip.core_id);
+}
+static DEVICE_ATTR_RO(core_id);
+
+static ssize_t
+config_id_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+	struct komeda_pipeline *pipe = mdev->pipelines[0];
+	union komeda_config_id config_id;
+	int i;
+
+	memset(&config_id, 0, sizeof(config_id));
+
+	config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
+	config_id.side_by_side = mdev->side_by_side;
+	config_id.n_pipelines = mdev->n_pipelines;
+	config_id.n_scalers = pipe->n_scalers;
+	config_id.n_layers = pipe->n_layers;
+	config_id.n_richs = 0;
+	for (i = 0; i < pipe->n_layers; i++) {
+		if (pipe->layers[i]->layer_type == KOMEDA_FMT_RICH_LAYER)
+			config_id.n_richs++;
+	}
+	return snprintf(buf, PAGE_SIZE, "0x%08x\n", config_id.value);
+}
+static DEVICE_ATTR_RO(config_id);
+
+static ssize_t
+aclk_hz_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%lu\n", clk_get_rate(mdev->aclk));
+}
+static DEVICE_ATTR_RO(aclk_hz);
+
+static struct attribute *komeda_sysfs_entries[] = {
+	&dev_attr_core_id.attr,
+	&dev_attr_config_id.attr,
+	&dev_attr_aclk_hz.attr,
+	NULL,
+};
+
+static struct attribute_group komeda_sysfs_attr_group = {
+	.attrs = komeda_sysfs_entries,
+};
+
+int komeda_dev_sysfs_init(struct komeda_dev *mdev)
+{
+	struct device *dev = mdev->dev;
+	int err;
+
+	err = sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
+	if (err)
+		DRM_ERROR("create sysfs group failed.\n");
+
+	return err;
+}
+
+void komeda_dev_sysfs_destroy(struct komeda_dev *mdev)
+{
+	struct device *dev = mdev->dev;
+
+	sysfs_remove_group(&dev->kobj, &komeda_sysfs_attr_group);
+}
-- 
2.20.1

[PATCH v1 2/2] drm/komeda: Refactor sysfs node "config_id"

[james qian wang (Arm Technology China)]

From: "James Qian Wang (Arm Technology China)" <james.qian.wang@arm.com>

Split sysfs config_id bitfiles to multiple separated sysfs files.

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../drm/arm/display/include/malidp_product.h  | 13 ---
 .../gpu/drm/arm/display/komeda/komeda_sysfs.c | 80 ++++++++++++++-----
 2 files changed, 62 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h b/drivers/gpu/drm/arm/display/include/malidp_product.h
index dbd3d4765065..b21f4aa15c95 100644
--- a/drivers/gpu/drm/arm/display/include/malidp_product.h
+++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
@@ -21,17 +21,4 @@
 #define MALIDP_D71_PRODUCT_ID	0x0071
 #define MALIDP_D32_PRODUCT_ID	0x0032
 
-union komeda_config_id {
-	struct {
-		__u32	max_line_sz:16,
-			n_pipelines:2,
-			n_scalers:2, /* number of scalers per pipeline */
-			n_layers:3, /* number of layers per pipeline */
-			n_richs:3, /* number of rich layers per pipeline */
-			side_by_side:1, /* if HW works on side_by_side mode */
-			reserved_bits:5;
-	};
-	__u32 value;
-};
-
 #endif /* _MALIDP_PRODUCT_H_ */
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
index 740f095b4ca5..5effab795dc1 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
@@ -18,28 +18,67 @@ core_id_show(struct device *dev, struct device_attribute *attr, char *buf)
 static DEVICE_ATTR_RO(core_id);
 
 static ssize_t
-config_id_show(struct device *dev, struct device_attribute *attr, char *buf)
+line_size_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct komeda_dev *mdev = dev_to_mdev(dev);
 	struct komeda_pipeline *pipe = mdev->pipelines[0];
-	union komeda_config_id config_id;
-	int i;
-
-	memset(&config_id, 0, sizeof(config_id));
-
-	config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
-	config_id.side_by_side = mdev->side_by_side;
-	config_id.n_pipelines = mdev->n_pipelines;
-	config_id.n_scalers = pipe->n_scalers;
-	config_id.n_layers = pipe->n_layers;
-	config_id.n_richs = 0;
-	for (i = 0; i < pipe->n_layers; i++) {
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", pipe->layers[0]->hsize_in.end);
+}
+static DEVICE_ATTR_RO(line_size);
+
+static ssize_t
+n_pipelines_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", mdev->n_pipelines);
+}
+static DEVICE_ATTR_RO(n_pipelines);
+
+static ssize_t
+n_layers_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+	struct komeda_pipeline *pipe = mdev->pipelines[0];
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", pipe->n_layers);
+}
+static DEVICE_ATTR_RO(n_layers);
+
+static ssize_t
+n_rich_layers_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+	struct komeda_pipeline *pipe = mdev->pipelines[0];
+	int i, n_richs = 0;
+
+	for (i = 0; i < pipe->n_layers; i++)
 		if (pipe->layers[i]->layer_type == KOMEDA_FMT_RICH_LAYER)
-			config_id.n_richs++;
-	}
-	return snprintf(buf, PAGE_SIZE, "0x%08x\n", config_id.value);
+			n_richs++;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", n_richs);
+}
+static DEVICE_ATTR_RO(n_rich_layers);
+
+static ssize_t
+n_scalers_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+	struct komeda_pipeline *pipe = mdev->pipelines[0];
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", pipe->n_scalers);
+}
+static DEVICE_ATTR_RO(n_scalers);
+
+static ssize_t
+side_by_side_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct komeda_dev *mdev = dev_to_mdev(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", mdev->side_by_side);
 }
-static DEVICE_ATTR_RO(config_id);
+static DEVICE_ATTR_RO(side_by_side);
 
 static ssize_t
 aclk_hz_show(struct device *dev, struct device_attribute *attr, char *buf)
@@ -52,7 +91,12 @@ static DEVICE_ATTR_RO(aclk_hz);
 
 static struct attribute *komeda_sysfs_entries[] = {
 	&dev_attr_core_id.attr,
-	&dev_attr_config_id.attr,
+	&dev_attr_line_size.attr,
+	&dev_attr_n_pipelines.attr,
+	&dev_attr_n_layers.attr,
+	&dev_attr_n_rich_layers.attr,
+	&dev_attr_n_scalers.attr,
+	&dev_attr_side_by_side.attr,
 	&dev_attr_aclk_hz.attr,
 	NULL,
 };
-- 
2.20.1

Re: [PATCH v1 2/2] drm/komeda: Refactor sysfs node "config_id"

[Daniel Vetter]

On Tue, Nov 26, 2019 at 10:54:47AM +0000, james qian wang (Arm Technology China) wrote:
> From: "James Qian Wang (Arm Technology China)" <james.qian.wang@arm.com>
> 
> Split sysfs config_id bitfiles to multiple separated sysfs files.
> 
> Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>

I guess Dave&my questions werent quite clear, this looks like uapi that's
consumed by hwc, so the userspace needs to be open source. Plus it needs
to be discussed/reviewed like any other kms uapi extensions, with a
critical eye whether this makes sense to add to a supposedly cross-vendor
interface.

I suspect the right thing to do here is to push the revert. From a quick
look at git history this landed together with the other kms properties in
komeda which we reverted already.
-Daniel

> ---
>  .../drm/arm/display/include/malidp_product.h  | 13 ---
>  .../gpu/drm/arm/display/komeda/komeda_sysfs.c | 80 ++++++++++++++-----
>  2 files changed, 62 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h b/drivers/gpu/drm/arm/display/include/malidp_product.h
> index dbd3d4765065..b21f4aa15c95 100644
> --- a/drivers/gpu/drm/arm/display/include/malidp_product.h
> +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
> @@ -21,17 +21,4 @@
>  #define MALIDP_D71_PRODUCT_ID	0x0071
>  #define MALIDP_D32_PRODUCT_ID	0x0032
>  
> -union komeda_config_id {
> -	struct {
> -		__u32	max_line_sz:16,
> -			n_pipelines:2,
> -			n_scalers:2, /* number of scalers per pipeline */
> -			n_layers:3, /* number of layers per pipeline */
> -			n_richs:3, /* number of rich layers per pipeline */
> -			side_by_side:1, /* if HW works on side_by_side mode */
> -			reserved_bits:5;
> -	};
> -	__u32 value;
> -};
> -
>  #endif /* _MALIDP_PRODUCT_H_ */
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
> index 740f095b4ca5..5effab795dc1 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
> @@ -18,28 +18,67 @@ core_id_show(struct device *dev, struct device_attribute *attr, char *buf)
>  static DEVICE_ATTR_RO(core_id);
>  
>  static ssize_t
> -config_id_show(struct device *dev, struct device_attribute *attr, char *buf)
> +line_size_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct komeda_dev *mdev = dev_to_mdev(dev);
>  	struct komeda_pipeline *pipe = mdev->pipelines[0];
> -	union komeda_config_id config_id;
> -	int i;
> -
> -	memset(&config_id, 0, sizeof(config_id));
> -
> -	config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
> -	config_id.side_by_side = mdev->side_by_side;
> -	config_id.n_pipelines = mdev->n_pipelines;
> -	config_id.n_scalers = pipe->n_scalers;
> -	config_id.n_layers = pipe->n_layers;
> -	config_id.n_richs = 0;
> -	for (i = 0; i < pipe->n_layers; i++) {
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", pipe->layers[0]->hsize_in.end);
> +}
> +static DEVICE_ATTR_RO(line_size);
> +
> +static ssize_t
> +n_pipelines_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct komeda_dev *mdev = dev_to_mdev(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", mdev->n_pipelines);
> +}
> +static DEVICE_ATTR_RO(n_pipelines);
> +
> +static ssize_t
> +n_layers_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct komeda_dev *mdev = dev_to_mdev(dev);
> +	struct komeda_pipeline *pipe = mdev->pipelines[0];
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", pipe->n_layers);
> +}
> +static DEVICE_ATTR_RO(n_layers);
> +
> +static ssize_t
> +n_rich_layers_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct komeda_dev *mdev = dev_to_mdev(dev);
> +	struct komeda_pipeline *pipe = mdev->pipelines[0];
> +	int i, n_richs = 0;
> +
> +	for (i = 0; i < pipe->n_layers; i++)
>  		if (pipe->layers[i]->layer_type == KOMEDA_FMT_RICH_LAYER)
> -			config_id.n_richs++;
> -	}
> -	return snprintf(buf, PAGE_SIZE, "0x%08x\n", config_id.value);
> +			n_richs++;
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", n_richs);
> +}
> +static DEVICE_ATTR_RO(n_rich_layers);
> +
> +static ssize_t
> +n_scalers_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct komeda_dev *mdev = dev_to_mdev(dev);
> +	struct komeda_pipeline *pipe = mdev->pipelines[0];
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", pipe->n_scalers);
> +}
> +static DEVICE_ATTR_RO(n_scalers);
> +
> +static ssize_t
> +side_by_side_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct komeda_dev *mdev = dev_to_mdev(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", mdev->side_by_side);
>  }
> -static DEVICE_ATTR_RO(config_id);
> +static DEVICE_ATTR_RO(side_by_side);
>  
>  static ssize_t
>  aclk_hz_show(struct device *dev, struct device_attribute *attr, char *buf)
> @@ -52,7 +91,12 @@ static DEVICE_ATTR_RO(aclk_hz);
>  
>  static struct attribute *komeda_sysfs_entries[] = {
>  	&dev_attr_core_id.attr,
> -	&dev_attr_config_id.attr,
> +	&dev_attr_line_size.attr,
> +	&dev_attr_n_pipelines.attr,
> +	&dev_attr_n_layers.attr,
> +	&dev_attr_n_rich_layers.attr,
> +	&dev_attr_n_scalers.attr,
> +	&dev_attr_side_by_side.attr,
>  	&dev_attr_aclk_hz.attr,
>  	NULL,
>  };
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v1 2/2] drm/komeda: Refactor sysfs node "config_id"

[james qian wang (Arm Technology China)]

On Tue, Nov 26, 2019 at 01:08:05PM +0100, Daniel Vetter wrote:
> On Tue, Nov 26, 2019 at 10:54:47AM +0000, james qian wang (Arm Technology China) wrote:
> > From: "James Qian Wang (Arm Technology China)" <james.qian.wang@arm.com>
> >
> > Split sysfs config_id bitfiles to multiple separated sysfs files.
> >
> > Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
>
> I guess Dave&my questions werent quite clear, this looks like uapi that's
> consumed by hwc, so the userspace needs to be open source. Plus it needs
> to be discussed/reviewed like any other kms uapi extensions, with a
> critical eye whether this makes sense to add to a supposedly cross-vendor
> interface.
>

Hi Dave & Daniel:

I think some komeda sysfs nodes can be added as cross-vendor.

  - core_id:
    which actually is like vendor_id/subsystem_device_id/revision in
    PCI dev, Maybe we can add a util funcs to fake these sysfs nodes
    for none PCI-dev like komeda

    device_info_sysfs_add(struct device *dev,
        u16 vendor, u16 subsystem_vendor,
        u16 subsystem_device, u8 revision);

    The arguments:
      vendor: I'd like to use the PCI vendor_ID, since with that user
              can see a unique ID, and easy to indentify different devices.
      subsystem_vendor/device/revision: device specific.

  - line_size.
    This actually is mode_config->max_width, but current drm still use
    this value to restrict the fb->width, but our HW supports crop, we
    don't have such limition. we can not set the real line_size to
    max_width.
    And I saw there is a fix:
    https://patchwork.freedesktop.org/patch/333454/
    with this fix, we can directly use mode_config->max_width

Beside that we still needs some komeda specific node like:
 - aclk_hz: for expose display engine clock.
 - num_scalers: per pipeline scalers
 - num_pipes: number of display pipelines.

For the open sourced user space, we're trying to switch to
drm_hwcomposer, and drop our internal hwcomposer. but that may need time.
Can we start from porting our specific test to IGT for komeda private uapi
coverage.

Thanks
James
> I suspect the right thing to do here is to push the revert. From a quick
> look at git history this landed together with the other kms properties in
> komeda which we reverted already.
> -Daniel
>
> > ---
> >  .../drm/arm/display/include/malidp_product.h  | 13 ---
> >  .../gpu/drm/arm/display/komeda/komeda_sysfs.c | 80 ++++++++++++++-----
> >  2 files changed, 62 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > index dbd3d4765065..b21f4aa15c95 100644
> > --- a/drivers/gpu/drm/arm/display/include/malidp_product.h
> > +++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
> > @@ -21,17 +21,4 @@
> >  #define MALIDP_D71_PRODUCT_ID      0x0071
> >  #define MALIDP_D32_PRODUCT_ID      0x0032
> >
> > -union komeda_config_id {
> > -   struct {
> > -           __u32   max_line_sz:16,
> > -                   n_pipelines:2,
> > -                   n_scalers:2, /* number of scalers per pipeline */
> > -                   n_layers:3, /* number of layers per pipeline */
> > -                   n_richs:3, /* number of rich layers per pipeline */
> > -                   side_by_side:1, /* if HW works on side_by_side mode */
> > -                   reserved_bits:5;
> > -   };
> > -   __u32 value;
> > -};
> > -
> >  #endif /* _MALIDP_PRODUCT_H_ */
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
> > index 740f095b4ca5..5effab795dc1 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_sysfs.c
> > @@ -18,28 +18,67 @@ core_id_show(struct device *dev, struct device_attribute *attr, char *buf)
> >  static DEVICE_ATTR_RO(core_id);
> >
> >  static ssize_t
> > -config_id_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +line_size_show(struct device *dev, struct device_attribute *attr, char *buf)
> >  {
> >     struct komeda_dev *mdev = dev_to_mdev(dev);
> >     struct komeda_pipeline *pipe = mdev->pipelines[0];
> > -   union komeda_config_id config_id;
> > -   int i;
> > -
> > -   memset(&config_id, 0, sizeof(config_id));
> > -
> > -   config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
> > -   config_id.side_by_side = mdev->side_by_side;
> > -   config_id.n_pipelines = mdev->n_pipelines;
> > -   config_id.n_scalers = pipe->n_scalers;
> > -   config_id.n_layers = pipe->n_layers;
> > -   config_id.n_richs = 0;
> > -   for (i = 0; i < pipe->n_layers; i++) {
> > +
> > +   return snprintf(buf, PAGE_SIZE, "%d\n", pipe->layers[0]->hsize_in.end);
> > +}
> > +static DEVICE_ATTR_RO(line_size);
> > +
> > +static ssize_t
> > +n_pipelines_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +   struct komeda_dev *mdev = dev_to_mdev(dev);
> > +
> > +   return snprintf(buf, PAGE_SIZE, "%d\n", mdev->n_pipelines);
> > +}
> > +static DEVICE_ATTR_RO(n_pipelines);
> > +
> > +static ssize_t
> > +n_layers_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +   struct komeda_dev *mdev = dev_to_mdev(dev);
> > +   struct komeda_pipeline *pipe = mdev->pipelines[0];
> > +
> > +   return snprintf(buf, PAGE_SIZE, "%d\n", pipe->n_layers);
> > +}
> > +static DEVICE_ATTR_RO(n_layers);
> > +
> > +static ssize_t
> > +n_rich_layers_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +   struct komeda_dev *mdev = dev_to_mdev(dev);
> > +   struct komeda_pipeline *pipe = mdev->pipelines[0];
> > +   int i, n_richs = 0;
> > +
> > +   for (i = 0; i < pipe->n_layers; i++)
> >             if (pipe->layers[i]->layer_type == KOMEDA_FMT_RICH_LAYER)
> > -                   config_id.n_richs++;
> > -   }
> > -   return snprintf(buf, PAGE_SIZE, "0x%08x\n", config_id.value);
> > +                   n_richs++;
> > +
> > +   return snprintf(buf, PAGE_SIZE, "%d\n", n_richs);
> > +}
> > +static DEVICE_ATTR_RO(n_rich_layers);
> > +
> > +static ssize_t
> > +n_scalers_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +   struct komeda_dev *mdev = dev_to_mdev(dev);
> > +   struct komeda_pipeline *pipe = mdev->pipelines[0];
> > +
> > +   return snprintf(buf, PAGE_SIZE, "%d\n", pipe->n_scalers);
> > +}
> > +static DEVICE_ATTR_RO(n_scalers);
> > +
> > +static ssize_t
> > +side_by_side_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +   struct komeda_dev *mdev = dev_to_mdev(dev);
> > +
> > +   return snprintf(buf, PAGE_SIZE, "%d\n", mdev->side_by_side);
> >  }
> > -static DEVICE_ATTR_RO(config_id);
> > +static DEVICE_ATTR_RO(side_by_side);
> >
> >  static ssize_t
> >  aclk_hz_show(struct device *dev, struct device_attribute *attr, char *buf)
> > @@ -52,7 +91,12 @@ static DEVICE_ATTR_RO(aclk_hz);
> >
> >  static struct attribute *komeda_sysfs_entries[] = {
> >     &dev_attr_core_id.attr,
> > -   &dev_attr_config_id.attr,
> > +   &dev_attr_line_size.attr,
> > +   &dev_attr_n_pipelines.attr,
> > +   &dev_attr_n_layers.attr,
> > +   &dev_attr_n_rich_layers.attr,
> > +   &dev_attr_n_scalers.attr,
> > +   &dev_attr_side_by_side.attr,
> >     &dev_attr_aclk_hz.attr,
> >     NULL,
> >  };
> > --
> > 2.20.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.