Re: [PATCH v14 1/2] drm: add kms driver for loongson display controller

[Sui Jingfeng]

Hi,

On 2023/5/21 20:21, WANG Xuerui wrote:
> > +++ b/drivers/gpu/drm/loongson/lsdc_debugfs.c
> > @@ -0,0 +1,91 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2023 Loongson Technology Corporation Limited
> > + */
> > +
> > +#include <drm/drm_debugfs.h>
> > +
> > +#include "lsdc_benchmark.h"
> > +#include "lsdc_drv.h"
> > +#include "lsdc_gem.h"
> > +#include "lsdc_probe.h"
> > +#include "lsdc_ttm.h"
> > +
> > +/* device level debugfs */
> > +
> > +static int lsdc_identify(struct seq_file *m, void *arg)
> > +{
> > +    struct drm_info_node *node = (struct drm_info_node *)m->private;
> > +    struct lsdc_device *ldev = (struct lsdc_device 
> > *)node->info_ent->data;
> > +    const struct loongson_gfx_desc *gfx = to_loongson_gfx(ldev->descp);
> > +    u8 impl, rev;
> > +
> > +    loongson_cpu_get_prid(&impl, &rev);
> > +
> > +    seq_printf(m, "Running on cpu 0x%x, cpu revision: 0x%x\n",
> > +           impl, rev);
>
> Is this really needed/relevant for LSDC identification? AFAICS the 
> loongson_cpu_get_prid helper has only one use (that's here), 

Yes, this is really needed, when doing the remote debugging, sometime 
you only have a ssh login the target machine.

User of the driver could know what the host is in the DRM way.

> so if it's not absolutely necessary you can just get rid of that 
> function and lsdc_probe.h altogether.
This function it written for the future, It will not be removed.
> > +
> > +    seq_printf(m, "Contained in: %s\n", gfx->model);
>
> "model: " would be more appropriate for a piece of info looking like a 
> "gfx->model"?
No, these are nearly equivalent.