All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: John.p.donnelly@oracle.com, dri-devel@lists.freedesktop.org,
	airlied@redhat.com, emil.velikov@collabora.com
Subject: Re: [PATCH 02/12] drm/mgag200: Return errno codes from PLL compute functions
Date: Mon, 12 Jul 2021 15:42:24 +0200	[thread overview]
Message-ID: <92d37579-0f5a-9f98-9c2c-4aeaedf38ac4@suse.de> (raw)
In-Reply-To: <YOibJbgFFfqQ6VqS@ravnborg.org>


[-- Attachment #1.1: Type: text/plain, Size: 1856 bytes --]

Hi

Am 09.07.21 um 20:53 schrieb Sam Ravnborg:
> On Mon, Jul 05, 2021 at 02:45:05PM +0200, Thomas Zimmermann wrote:
>> Return -EINVAL if there's no PLL configuration for the given pixel
>> clock.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 482843ebb69f..045a20055515 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -134,7 +134,7 @@ static int mgag200_g200_set_plls(struct mga_device *mdev, long clock)
>>   
>>   	if (clock > p_clk_max) {
>>   		drm_err(dev, "Pixel Clock %ld too high\n", clock);
>> -		return 1;
>> +		return -EINVAL;
>>   	}
>>   
>>   	if (clock < p_clk_min >> 3)
>> @@ -293,7 +293,7 @@ static int mga_g200se_set_plls(struct mga_device *mdev, long clock)
>>   
>>   	if (delta > permitteddelta) {
>>   		pr_warn("PLL delta too large\n");
>> -		return 1;
>> +		return -EINVAL;
>>   	}
>>   
>>   	misc = RREG8(MGA_MISC_IN);
> 
> The return value is ignored but I assume it makes sense in a later
> patch. Should mgag200_crtc_set_plls() return -EINVAL if there was no
> match? Today it returns 0 - which is not an error.

Indeed. Patch 12 moves some of this functionality into the atomic check, 
where it will be tested for success.

Not handling the type in the switch is actually a driver bug. I'll see 
if I can add a rsp error in one of the patches.

Best regards
Thomas

> 
> 	Sam
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

  reply	other threads:[~2021-07-12 13:42 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05 12:45 [PATCH 00/12] mgag200: Refactor PLL setup Thomas Zimmermann
2021-07-05 12:45 ` [PATCH 01/12] drm/mgag200: Select clock in PLL update functions Thomas Zimmermann
2021-07-05 12:45   ` Thomas Zimmermann
2021-07-09 18:50   ` Sam Ravnborg
2021-07-12 13:36     ` Thomas Zimmermann
2021-07-12 13:36       ` Thomas Zimmermann
2021-07-05 12:45 ` [PATCH 02/12] drm/mgag200: Return errno codes from PLL compute functions Thomas Zimmermann
2021-07-09 18:53   ` Sam Ravnborg
2021-07-12 13:42     ` Thomas Zimmermann [this message]
2021-07-05 12:45 ` [PATCH 03/12] drm/mgag200: Remove P_ARRAY_SIZE Thomas Zimmermann
2021-07-09 18:53   ` Sam Ravnborg
2021-07-05 12:45 ` [PATCH 04/12] drm/mgag200: Split PLL setup into compute and update functions Thomas Zimmermann
2021-07-09 19:12   ` Sam Ravnborg
2021-07-12 14:03     ` Thomas Zimmermann
2021-07-05 12:45 ` [PATCH 05/12] drm/mgag200: Introduce separate variable for PLL S parameter Thomas Zimmermann
2021-07-09 19:18   ` Sam Ravnborg
2021-07-05 12:45 ` [PATCH 06/12] drm/mgag200: Store values (not bits) in struct mgag200_pll_values Thomas Zimmermann
2021-07-10  7:06   ` Sam Ravnborg
2021-07-12 14:09     ` Thomas Zimmermann
2021-07-12 14:18       ` Sam Ravnborg
2021-07-05 12:45 ` [PATCH 07/12] drm/mgag200: Split several PLL functions by device type Thomas Zimmermann
2021-07-05 12:45 ` [PATCH 08/12] drm/mgag200: Separate PLL compute and update functions from each other Thomas Zimmermann
2021-07-05 12:45 ` [PATCH 09/12] drm/mgag200: Split PLL computation for G200SE Thomas Zimmermann
2021-07-05 12:45 ` [PATCH 10/12] drm/mgag200: Declare PLL clock constants static const Thomas Zimmermann
2021-07-05 12:45 ` [PATCH 11/12] drm/mgag200: Introduce custom CRTC state Thomas Zimmermann
2021-07-10  7:01   ` Sam Ravnborg
2021-07-05 12:45 ` [PATCH 12/12] drm/mgag200: Compute PLL values during atomic check Thomas Zimmermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=92d37579-0f5a-9f98-9c2c-4aeaedf38ac4@suse.de \
    --to=tzimmermann@suse.de \
    --cc=John.p.donnelly@oracle.com \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.velikov@collabora.com \
    --cc=sam@ravnborg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.