Re: [PATCH 6/7] hw/display/artist: Fix artist screen resolution

[Richard Henderson]

On 9/2/20 2:48 PM, Helge Deller wrote:
>> Is it a bug that these Y were using 0x3ff and not 0x7ff?
>> Because it's pretty consistent...
> 
> I'm not sure if was intentional, a bug or just initial coding by Sven.
> The old code was limiting to 1024 lines.
> I assume that there doesn't exist any real hardware with > 1024 lines,
> so it's more theoretical.
> I tested the code, and it seems to work with > 1024 at least.
> So, I think to use 0x7ff is probably more correct - at least if we
> allow more lines.
> 
>> You should make that a separate change, for sure.
> 
> I'd prefer to keep it in one patch.
> It's simply moving code around and reuse a macro.

It's more than that, it's a behaviour change.

Code refactoring should be separate from feature changes, when at all possible.
 And here, its certainly possible.

>> Was the original values chosen by the user?
> 
> Yes, can bet set by user via
> -global -global artist.width=800 -global artist.height=600
> 
>> Should we be giving some sort of error for out-of-range values?
> 
> I had a warning there, but then removed it again. Most users will
> probably use the default values anyway. If you really want, I will add a
> warning. I'm not a fan of errors. Errors usually abort, but we can
> adjust and simply continue here.

It's exactly the same error as -smp 1234, when only 8 cpus are supported.  An
error is more than appropriate.


r~