[MPlayer-dev-eng] [PATCH] nvidia_vid update

[Sascha Sommer]

>> > -    unsigned int next_frame;
>> > +    unsigned int cur_frame;
>>
>> Looks like pure cosmetics. Is this really needed?
>
>It's cosmetics :) But IMHO a bit of cosmetics is better, than next_frame
>variable pointing to current frame and not to the next one.

Dunno and don't care. Feel free to commit.

>According to nv4ref.h, only 2 last bits define bpp:
>/* Controls what the format of the framebuffer is */
>#define NV_PCRTC_PIXEL                       0x28
>#define NV_PCRTC_PIXEL_MODE                  7:7
>#define NV_PCRTC_PIXEL_MODE_TV               0x01
>#define NV_PCRTC_PIXEL_MODE_VGA              0x00
>#define NV_PCRTC_PIXEL_TV_MODE               6:6
>#define NV_PCRTC_PIXEL_TV_MODE_NTSC          0x00
>#define NV_PCRTC_PIXEL_TV_MODE_PAL           0x01
>#define NV_PCRTC_PIXEL_TV_HORIZ_ADJUST       5:3
>#define NV_PCRTC_PIXEL_FORMAT                1:0
>#define NV_PCRTC_PIXEL_FORMAT_VGA            0x00
>#define NV_PCRTC_PIXEL_FORMAT_8BPP           0x01
>#define NV_PCRTC_PIXEL_FORMAT_16BPP          0x02
>#define NV_PCRTC_PIXEL_FORMAT_32BPP          0x03

Seems to be the same for nv03. Can someone please test
with Geforce.

>> >   /* get 9th bit in CRTC_OVERFLOW*/
>> >   info->screen_y |= (VID_RD08(info->chip.PCIO,0x03D5) &0x02)<<7;
>> >   /* and the 10th in CRTC_OVERFLOW*/
>> >+  VID_WR08(info->chip.PCIO,0x03D4,0x07);
>>
>> the register shouldn't change. Does it?
>
>Just to be sure. At least it may change (multitasking :).

I'm against commiting this change. If it would change
between the two reads it might aswell change between
the write and the read.

>> >+ while (bps == 0)
>> >+ {
>> >+  if (info->chip.arch != NV_ARCH_03)
>> >+   bps = info->chip.PGRAPH[0x00000670/4];
>> >+  else
>> >+   bps = info->chip.PGRAPH[0x00000650/4];
>> >+//  if (bps == 0) fprintf(stderr, "bps returned 0: %d\n");
>> >+ }
>> >      x = info->wx - (pan % bps) * 8 / info->depth;
>> >      y = info->wy - (pan / bps);
>> >+    }
>> >+
>>
>> I'm currently unable to test it, but afair this register is also 0 in
>> textmode.
>
>Yes, but in text mode info->depth == 0, so the whole block is skipped.

Please leave this code as it is now unlike you can give me a reason why your
changes are needed.


Sascha